From 24c48bc3eaee4ed0ab9c6da237b9e87516a8feea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 2 Feb 2024 12:09:30 -0300 Subject: [PATCH] feat: add search highlight/expand and "no tags" message (#799) This change makes minor improvements in the search taxonomy UI. It expands taxonomies that match the search and highlight the search term, and adds a "No tag found with search term '....'" message. --- .../ContentTagsCollapsible.jsx | 3 +- .../ContentTagsCollapsible.test.jsx | 8 +- .../ContentTagsDropDownSelector.jsx | 165 +++++++++++------- .../ContentTagsDropDownSelector.test.jsx | 79 +++++++-- src/content-tags-drawer/data/api.js | 6 +- src/content-tags-drawer/data/api.test.js | 5 +- src/content-tags-drawer/data/apiHooks.jsx | 24 ++- .../data/apiHooks.test.jsx | 14 +- src/content-tags-drawer/messages.js | 4 + 9 files changed, 201 insertions(+), 107 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 470b38f9c..82ffda249 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -123,7 +123,8 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData }) => { const handleSearchChange = React.useCallback((value) => { if (value === '') { - // No need to debounce when search term cleared + // No need to debounce when search term cleared. Clear debounce function + handleSearch.cancel(); setSearchTerm(''); } else { handleSearch(value); diff --git a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx index 09a28f55d..772087c18 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx @@ -18,12 +18,12 @@ jest.mock('./data/apiHooks', () => ({ })), useTaxonomyTagsData: jest.fn(() => ({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: true, isError: false, canAddTag: false, data: [], - }], + }, })), })); @@ -85,7 +85,7 @@ describe('', () => { useTaxonomyTagsData.mockReturnValue({ hasMorePages: false, canAddTag: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -119,7 +119,7 @@ describe('', () => { canChangeTag: false, canDeleteTag: false, }], - }], + }, }); } diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx index bd9df6af2..5e1053555 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx @@ -1,5 +1,5 @@ // @ts-check -import React, { useState, useCallback } from 'react'; +import React, { useEffect, useState, useCallback } from 'react'; import { SelectableBox, Icon, @@ -14,6 +14,33 @@ import './ContentTagsDropDownSelector.scss'; import { useTaxonomyTagsData } from './data/apiHooks'; +const HighlightedText = ({ text, highlight }) => { + if (!highlight) { + return {text}; + } + + const parts = text.split(new RegExp(`(${highlight})`, 'gi')); + return ( + + {parts.map((part, index) => ( + // eslint-disable-next-line react/no-array-index-key -- using index because part is not unique + + {part.toLowerCase() === highlight.toLowerCase() ? {part} : part} + + ))} + + ); +}; + +HighlightedText.propTypes = { + text: PropTypes.string.isRequired, + highlight: PropTypes.string, +}; + +HighlightedText.defaultProps = { + highlight: '', +}; + const ContentTagsDropDownSelector = ({ taxonomyId, level, lineage, tagsTree, searchTerm, }) => { @@ -22,7 +49,7 @@ const ContentTagsDropDownSelector = ({ // This object represents the states of the dropdowns on this level // The keys represent the index of the dropdown with // the value true (open) false (closed) - const [dropdownStates, setDropdownStates] = useState({}); + const [dropdownStates, setDropdownStates] = useState(/** type Record */ {}); const isOpen = (tagValue) => dropdownStates[tagValue]; const [numPages, setNumPages] = useState(1); @@ -38,6 +65,23 @@ const ContentTagsDropDownSelector = ({ setNumPages(1); } + useEffect(() => { + if (tagPages.isSuccess) { + if (searchTerm) { + const expandAll = tagPages.data.reduce( + (acc, tagData) => ({ + ...acc, + [tagData.value]: !!tagData.childCount, + }), + {}, + ); + setDropdownStates(expandAll); + } else { + setDropdownStates({}); + } + } + }, [searchTerm, tagPages.isSuccess]); + const clickAndEnterHandler = (tagValue) => { // This flips the state of the dropdown at index false (closed) -> true (open) // and vice versa. Initially they are undefined which is falsy. @@ -60,69 +104,62 @@ const ContentTagsDropDownSelector = ({ return (
- {tagPages.map((tagPage, pageNum) => ( - // Array index represents the page number - // eslint-disable-next-line react/no-array-index-key - - {tagPage.isLoading ? ( -
- -
- ) : null } - {tagPage.isError ? 'Error...' : null /* TODO: show a proper error message */} + {tagPages.isLoading ? ( +
+ +
+ ) : null } + {tagPages.isError ? 'Error...' : null /* TODO: show a proper error message */} - {tagPage.data?.map((tagData) => ( - -
( + +
+
+ encodeURIComponent(t)).join(',')} + isIndeterminate={isImplicit(tagData)} + disabled={isImplicit(tagData)} > -
- encodeURIComponent(t)).join(',')} - isIndeterminate={isImplicit(tagData)} - disabled={isImplicit(tagData)} - > - {tagData.value} - - { tagData.childCount > 0 - && ( -
- clickAndEnterHandler(tagData.value)} - tabIndex="0" - onKeyPress={(event) => (event.key === 'Enter' ? clickAndEnterHandler(tagData.value) : null)} - /> -
- )} -
+ + + { tagData.childCount > 0 + && ( +
+ clickAndEnterHandler(tagData.value)} + tabIndex="0" + onKeyPress={(event) => (event.key === 'Enter' ? clickAndEnterHandler(tagData.value) : null)} + /> +
+ )} +
-
+
- { tagData.childCount > 0 && isOpen(tagData.value) && ( - - )} - - - ))} + { tagData.childCount > 0 && isOpen(tagData.value) && ( + + )} ))} @@ -141,6 +178,12 @@ const ContentTagsDropDownSelector = ({ ) : null} + { tagPages.data.length === 0 && !tagPages.isLoading && ( +
+ +
+ )} +
); }; diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx index 8f25e40a2..7800e99e7 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx @@ -13,11 +13,11 @@ import { useTaxonomyTagsData } from './data/apiHooks'; jest.mock('./data/apiHooks', () => ({ useTaxonomyTagsData: jest.fn(() => ({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: true, isError: false, data: [], - }], + }, })), })); @@ -70,7 +70,7 @@ describe('', () => { it('should render taxonomy tags drop down selector with no sub tags', async () => { useTaxonomyTagsData.mockReturnValue({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -82,7 +82,7 @@ describe('', () => { id: 12345, subTagsUrl: null, }], - }], + }, }); await act(async () => { @@ -104,7 +104,7 @@ describe('', () => { it('should render taxonomy tags drop down selector with sub tags', async () => { useTaxonomyTagsData.mockReturnValue({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -116,7 +116,7 @@ describe('', () => { id: 12345, subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=Tag%202', }], - }], + }, }); await act(async () => { @@ -137,7 +137,7 @@ describe('', () => { it('should expand on click taxonomy tags drop down selector with sub tags', async () => { useTaxonomyTagsData.mockReturnValueOnce({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -149,7 +149,7 @@ describe('', () => { id: 12345, subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=Tag%202', }], - }], + }, }); await act(async () => { @@ -177,7 +177,7 @@ describe('', () => { // Mock useTaxonomyTagsData again since it gets called in the recursive call useTaxonomyTagsData.mockReturnValueOnce({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -189,7 +189,7 @@ describe('', () => { id: 12346, subTagsUrl: null, }], - }], + }, }); // Expand the dropdown to see the subtags selectors @@ -205,7 +205,7 @@ describe('', () => { it('should expand on enter key taxonomy tags drop down selector with sub tags', async () => { useTaxonomyTagsData.mockReturnValueOnce({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -217,7 +217,7 @@ describe('', () => { id: 12345, subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=Tag%202', }], - }], + }, }); await act(async () => { @@ -245,7 +245,7 @@ describe('', () => { // Mock useTaxonomyTagsData again since it gets called in the recursive call useTaxonomyTagsData.mockReturnValueOnce({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, data: [{ @@ -257,7 +257,7 @@ describe('', () => { id: 12346, subTagsUrl: null, }], - }], + }, }); // Expand the dropdown to see the subtags selectors @@ -273,9 +273,10 @@ describe('', () => { it('should render taxonomy tags drop down selector and change search term', async () => { useTaxonomyTagsData.mockReturnValueOnce({ hasMorePages: false, - tagPages: [{ + tagPages: { isLoading: false, isError: false, + isSuccess: true, data: [{ value: 'Tag 1', externalId: null, @@ -285,7 +286,7 @@ describe('', () => { id: 12345, subTagsUrl: null, }], - }], + }, }); const initalSearchTerm = 'test 1'; @@ -316,6 +317,52 @@ describe('', () => { await waitFor(() => { expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, updatedSearchTerm); }); + + // Clean search term + const cleanSearchTerm = ''; + rerender(); + + await waitFor(() => { + expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, cleanSearchTerm); + }); + }); + }); + + it('should render "noTag" message if search doesnt return taxonomies', async () => { + useTaxonomyTagsData.mockReturnValueOnce({ + hasMorePages: false, + tagPages: { + isLoading: false, + isError: false, + isSuccess: true, + data: [], + }, + }); + + const searchTerm = 'uncommon search term'; + await act(async () => { + const { getByText } = render( + , + ); + + await waitFor(() => { + expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, searchTerm); + }); + + const message = `No tags found with the search term "${searchTerm}"`; + expect(getByText(message)).toBeInTheDocument(); }); }); }); diff --git a/src/content-tags-drawer/data/api.js b/src/content-tags-drawer/data/api.js index 28bd7a36c..d5a4f3de1 100644 --- a/src/content-tags-drawer/data/api.js +++ b/src/content-tags-drawer/data/api.js @@ -75,8 +75,8 @@ export async function getContentData(contentId) { * @returns {Promise} */ export async function updateContentTaxonomyTags(contentId, taxonomyId, tags) { - const url = getContentTaxonomyTagsApiUrl(contentId); - const params = { taxonomy: taxonomyId }; - const { data } = await getAuthenticatedHttpClient().put(url, { tags }, { params }); + let url = getContentTaxonomyTagsApiUrl(contentId); + url = `${url}&taxonomy=${taxonomyId}`; + const { data } = await getAuthenticatedHttpClient().put(url, { tags }); return camelCaseObject(data[contentId]); } diff --git a/src/content-tags-drawer/data/api.test.js b/src/content-tags-drawer/data/api.test.js index 1ac441c2f..229e6daee 100644 --- a/src/content-tags-drawer/data/api.test.js +++ b/src/content-tags-drawer/data/api.test.js @@ -110,11 +110,10 @@ describe('content tags drawer api calls', () => { const contentId = 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb0b'; const taxonomyId = 3; const tags = ['flat taxonomy tag 100', 'flat taxonomy tag 3856']; - axiosMock.onPut(`${getContentTaxonomyTagsApiUrl(contentId)}`).reply(200, updateContentTaxonomyTagsMock); + axiosMock.onPut(`${getContentTaxonomyTagsApiUrl(contentId)}&taxonomy=${taxonomyId}`).reply(200, updateContentTaxonomyTagsMock); const result = await updateContentTaxonomyTags(contentId, taxonomyId, tags); - expect(axiosMock.history.put[0].url).toEqual(`${getContentTaxonomyTagsApiUrl(contentId)}`); - expect(axiosMock.history.put[0].params).toEqual({ taxonomy: taxonomyId }); + expect(axiosMock.history.put[0].url).toEqual(`${getContentTaxonomyTagsApiUrl(contentId)}&taxonomy=${taxonomyId}`); expect(result).toEqual(updateContentTaxonomyTagsMock[contentId]); }); }); diff --git a/src/content-tags-drawer/data/apiHooks.jsx b/src/content-tags-drawer/data/apiHooks.jsx index 869cb8280..1b95f6093 100644 --- a/src/content-tags-drawer/data/apiHooks.jsx +++ b/src/content-tags-drawer/data/apiHooks.jsx @@ -22,14 +22,6 @@ import { * @param {string|null} parentTag The tag whose children we're loading, if any * @param {string} searchTerm The term passed in to perform search on tags * @param {number} numPages How many pages of tags to load at this level - * @returns {{ - * hasMorePages: boolean, - * tagPages: { - * isLoading: boolean, - * isError: boolean, - * data: TagData[], - * }[], - * }} */ export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1, searchTerm = '') => { const queryClient = useQueryClient(); @@ -53,13 +45,10 @@ export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1, const hasMorePages = numPages < totalPages; const tagPages = useMemo(() => { - /** @type { { isLoading: boolean, isError: boolean, data: TagData[] }[] } */ - const newTags = []; - // Pre-load desendants if possible const preLoadedData = new Map(); - dataPages.forEach(result => { + const newTags = dataPages.map(result => { /** @type {TagData[]} */ const simplifiedTagsList = []; @@ -73,7 +62,7 @@ export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1, } }); - newTags.push({ ...result, data: simplifiedTagsList }); + return { ...result, data: simplifiedTagsList }; }); // Store the pre-loaded descendants into the query cache: @@ -95,7 +84,14 @@ export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1, return newTags; }, [dataPages]); - return { hasMorePages, tagPages }; + const flatTagPages = { + isLoading: tagPages.some(page => page.isLoading), + isError: tagPages.some(page => page.isError), + isSuccess: tagPages.every(page => page.isSuccess), + data: tagPages.flatMap(page => page.data), + }; + + return { hasMorePages, tagPages: flatTagPages }; }; /** diff --git a/src/content-tags-drawer/data/apiHooks.test.jsx b/src/content-tags-drawer/data/apiHooks.test.jsx index 8000a424a..4e12ef5ea 100644 --- a/src/content-tags-drawer/data/apiHooks.test.jsx +++ b/src/content-tags-drawer/data/apiHooks.test.jsx @@ -67,9 +67,12 @@ describe('useTaxonomyTagsData', () => { ], }; - useQueries.mockReturnValue([ - { data: mockData, isLoading: false, isError: false }, - ]); + useQueries.mockReturnValue([{ + data: mockData, + isLoading: false, + isError: false, + isSuccess: true, + }]); const { result } = renderHook(() => useTaxonomyTagsData(taxonomyId)); @@ -83,10 +86,11 @@ describe('useTaxonomyTagsData', () => { expect(result.current.hasMorePages).toEqual(false); // Only includes the first 2 tags because the other 2 would be // in the nested dropdown - expect(result.current.tagPages).toEqual([ + expect(result.current.tagPages).toEqual( { isLoading: false, isError: false, + isSuccess: true, data: [ { value: 'tag 1', @@ -108,7 +112,7 @@ describe('useTaxonomyTagsData', () => { }, ], }, - ]); + ); }); }); diff --git a/src/content-tags-drawer/messages.js b/src/content-tags-drawer/messages.js index 62d9e1549..c54e6b7bc 100644 --- a/src/content-tags-drawer/messages.js +++ b/src/content-tags-drawer/messages.js @@ -21,6 +21,10 @@ const messages = defineMessages({ id: 'course-authoring.content-tags-drawer.tags-dropdown-selector.load-more-tags.button', defaultMessage: 'Load more', }, + noTagsFoundMessage: { + id: 'course-authoring.content-tags-drawer.tags-dropdown-selector.no-tags-found', + defaultMessage: 'No tags found with the search term "{searchTerm}"', + }, taxonomyTagsCheckboxAriaLabel: { id: 'course-authoring.content-tags-drawer.tags-dropdown-selector.selectable-box.aria.label', defaultMessage: '{tag} checkbox',