feat: example of pre-loading tags #18
Conversation
This commit adds the editing functionality for content tags (minus commuincation with the backend). You are able to select/deselect tags from the dropdowns in each taxonomy and the tags tree structure is updated and the state persists. When a tag is select, all its ancestors automatically become 'implicit' and are unselected and connot be selected. If an explicit tag is unselected and the parent (or ancestor) no longer has explicit children, all ancestor tags will also be removed.
This makes use of useMutation to communicate changes to the applied tags to the backend so they are updated in the DB. When a selectable box is checked/unchecked a request is made. And when an explicit tag is clicked on, a request is also made.
Also refactor tag change handler and wrap it with useCallback
Currently set to always be editable. Need to implement proper permission checks to determine whether user has access to readonly/editable drawer.
This is because we invalidate the query when the mutation fails (or succeeds) so we get the latest applied tags from the backend. So we clear the checkboxes and applied tags to reflect the latest state from the backend.
| @@ -1,3 +1,4 @@ | |||
| // @ts-check | |||
There was a problem hiding this comment.
Put this here and if you use VS Code, you'll get a lot more useful debugging errors/warnings from the TypeScript engine while editing this file!! Not sure how you survived without it :p
There was a problem hiding this comment.
I was surprised I didn't face any type errors but it was too good to be true 😂 Completely forgot to add this, all the red lines showed up.
| <> {/* <<<< TODO: make this into a <div> and put the left padding here so it affects everything - spinners, load more, tags, ... */} | ||
| {tagPages.map((tagPage, pageNum) => <React.Fragment key={pageNum}> | ||
| {tagPage.isLoading ? "Loading..." : null /* TODO: Use <LoadingSpinner /> from master src/components/generic */} | ||
| {tagPage.isError ? "Error..." : null /* TODO: show a proper error message */} |
There was a problem hiding this comment.
Note a bunch of TODOs ^
| const loadMoreTags = useCallback(() => { | ||
| setCurrentPage(currentPage + 1); | ||
| }, [currentPage]); | ||
| setNumPages((x) => x + 1); |
There was a problem hiding this comment.
When using a set function from a useState hook, you can pass in a function that gets the current value. This lets you simplify actions like incrementing. Now the useCallback actually has no dependencies :)
| @@ -60,42 +60,22 @@ | |||
|
|
|||
| /** | |||
| * @typedef {Object} TaxonomyTagData | |||
There was a problem hiding this comment.
Shouldn't this type be shared with the taxonomy list page? Also, it was totally wrong here :p
There was a problem hiding this comment.
Oops, I think that was the type before the improvements were made to the API, and I missed changing this. Now I know why I didn't get any errors lol
| const [prevSearchTerm, setPrevSearchTerm] = useState(searchTerm); | ||
|
|
||
| // Reset the page and tags state when search term changes | ||
| // and store search term to compare | ||
| if (prevSearchTerm !== searchTerm) { | ||
| setPrevSearchTerm(searchTerm); | ||
| setCurrentPage(1); | ||
| setNumPages(1); | ||
| closeAllDropdowns(); |
There was a problem hiding this comment.
^ After thinking about this, now that I refactored the dropdowns to remember which tags are expanded or not by ID rather than page number, I think you should preserve the state even when the searchTerm changes, so just get rid of closeAllDropdowns. It will be much more convenient/consistent for users if the tags that they've opened stay open even if the search term changes.
There was a problem hiding this comment.
I like this approach, much cleaner and allows more flexibility with keeping the dropdown opens.
| <ContentTagsDropDownSelector | ||
| taxonomyId={taxonomyId} | ||
| level={level + 1} | ||
| lineage={[...lineage, encodeURIComponent(tagData.value)]} |
There was a problem hiding this comment.
Nit: Why are you using encodeURIComponent with an array? I would think that with the array separating the string values, there is no need for encoding/decoding here.
There was a problem hiding this comment.
Because the value prop of the SelectableBox which needs to be a string, hence the comma separated encoded tags there, so each level of the lineage also needed to be encoded. However you're right, I can remove the encoding/decoding from here, and do it all in the value prop of SelectableBox like:
value={[...lineage, taxonomyTag.value].map(t => encodeURIComponent(t)).join(',')}
yusuf-musleh
force-pushed
the
yusuf-musleh/search-content-tags
branch
10 times, most recently
from
e964bde to
b118be9
Compare
bradenmacdonald
force-pushed
the
yusuf-musleh/search-content-tags
branch
2 times, most recently
from
1ee474a to
01c9cd1
Compare
yusuf-musleh
force-pushed
the
yusuf-musleh/search-content-tags
branch
4 times, most recently
from
f5d4c66 to
e989c48
Compare
@yusuf-musleh Here you go.