feat(web): #872 bulk tag deletion #900
Conversation
| return () => { | ||
| setVisibleTags([]); | ||
| }; | ||
| }, [initialData]); |
There was a problem hiding this comment.
hmmm, I don't think you should depend on initial data because it means you'll miss the updates after the first server side rendering. You should depend on data that's coming out of react query later in the function.
| const pathname = usePathname(); | ||
| const [currentPathname, setCurrentPathname] = useState(""); | ||
|
|
||
| useEffect(() => { | ||
| if (pathname !== currentPathname) { | ||
| setCurrentPathname(pathname); | ||
| setIsBulkEditEnabled(false); | ||
| } | ||
| }, [pathname, currentPathname]); |
There was a problem hiding this comment.
Should we maybe here do an unmount hook instead?
There was a problem hiding this comment.
You are right. Fixed in ffef39a
| onPressedChange={setIsBulkEditEnabled} | ||
| > | ||
| <Pencil className="mr-2 size-4" /> | ||
| Bulk Edit |
There was a problem hiding this comment.
We have a translation for this in actions.bulk_edit that we can use.
There was a problem hiding this comment.
Thank you! Change it to use translation in ffef39a
| <InfoTooltip size={15} className="my-auto ml-2" variant="explain"> | ||
| <p>Bulk Edit Tags</p> | ||
| </InfoTooltip> |
There was a problem hiding this comment.
This is probably redundant, we can remove it
There was a problem hiding this comment.
Removed in ffef39a
apps/web/lib/bulkTagActions.ts
Outdated
| selectedTags: ZTagBasic[]; | ||
| visibleTags: ZTagBasic[]; |
There was a problem hiding this comment.
Does the store actually care about the name of the tag or is the ids enough?
There was a problem hiding this comment.
No we don't need the name. Fixed it in ffef39a
Thank you for your review! That's totally fine, if there's any issue please feel free to request other changes |
|
Thanks a lot @lexafaxine for addressing the feedback and for the PR! |
#872
Add bulk tag deletion feature
Changes
Demo video
https://drive.google.com/file/d/192R-6t2cZPy2g8Dn_9BIRtrONs-Ir38z/view?usp=sharing