Skip to content

Commit

Permalink
refactor: Nits from PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
yusuf-musleh committed Nov 29, 2023
1 parent 9fd8c6e commit 4bdb8e8
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 15 deletions.
26 changes: 12 additions & 14 deletions src/content-tags-drawer/ContentTagsCollapsible.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -178,16 +178,13 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>
id, name, contentTags,
} = taxonomyAndTagsData;

// State to determine whether to update the backend or not
// State to determine whether the tags are being updating so we can make a call
// to the update endpoint to the reflect those changes
const [updatingTags, setUpdatingTags] = React.useState(false);
const mutation = useContentTaxonomyTagsMutation(contentId, id);

const [isOpen, open, close] = useToggle(false);
const [target, setTarget] = React.useState(null);

// Keeps track of the tree structure for the applied content tags passed
// in as a prop.
const [appliedContentTags, setAppliedContentTags] = React.useState({});
const [addTagsButtonRef, setAddTagsButtonRef] = React.useState(null);

// Keeps track of the tree structure for tags that are add by selecting/unselecting
// tags in the dropdowns.
Expand All @@ -196,10 +193,10 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>
// To handle checking/unchecking tags in the SelectableBox
const [checkedTags, { add, remove, clear }] = useCheckboxSetValues();

// Handles make requests to the backend whenever the checked tags change
// Handles making requests to the update endpoint whenever the checked tags change
React.useEffect(() => {
// We have this check because this hook is fired when the component first load
// and reloads (on refocus). We only want to make requests to the backend when
// We have this check because this hook is fired when the component first loads
// and reloads (on refocus). We only want to make a request to the update endpoint when
// the user is updating the tags.
if (updatingTags) {
setUpdatingTags(false);
Expand All @@ -209,7 +206,7 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>
}, [contentId, id, checkedTags]);

// This converts the contentTags prop to the tree structure mentioned above
React.useMemo(() => {
const appliedContentTags = React.useMemo(() => {
// Clear all the tags that have not been commited and the checked boxes when
// fresh contentTags passed in so the latest state from the backend is rendered
setAddedContentTags({});
Expand Down Expand Up @@ -243,8 +240,9 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>
});
});

setAppliedContentTags(resultTree);
}, [contentTags, setAddedContentTags, mutation.isError]);
// setAppliedContentTags(resultTree);
return resultTree;
}, [contentTags, mutation.isError]);

// This is the source of truth that represents the current state of tags in
// this Taxonomy as a tree. Whenever either the `appliedContentTags` (i.e. tags passed in
Expand Down Expand Up @@ -315,7 +313,7 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>

{editable && (
<Button
ref={setTarget}
ref={setAddTagsButtonRef}
variant="outline-primary"
onClick={open}
>
Expand All @@ -325,7 +323,7 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>
</div>
<ModalPopup
placement="bottom"
positionRef={target}
positionRef={addTagsButtonRef}
isOpen={isOpen}
onClose={close}
>
Expand Down
4 changes: 4 additions & 0 deletions src/content-tags-drawer/ContentTagsDropDownSelector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ const ContentTagsDropDownSelector = ({
// however if it is null that means it is the root, and the apiHooks
// would automatically handle it. Later this url is set to the next
// page of results (if any)
//
// TODO: In the future we may need to refactor this to keep track
// of the count for how many times the user clicked on "load more" then
// use useQueries to load all the pages based on that.
const [fetchUrl, setFetchUrl] = useState(subTagsUrl);

const isOpen = (i) => dropdownStates[i];
Expand Down
2 changes: 1 addition & 1 deletion src/content-tags-drawer/TagBubble.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const TagBubble = ({
if (!implicit && editable) {
removeTagHandler(lineage.join(','), false);
}
}, [implicit, lineage]);
}, [implicit, lineage, editable, removeTagHandler]);

return (
<div style={{ paddingLeft: `${level * 1}rem` }}>
Expand Down

0 comments on commit 4bdb8e8

Please sign in to comment.