From f370b565c28cd114ed559c154e3715fa311b9d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Sat, 17 Aug 2024 20:43:27 -0300 Subject: [PATCH 1/8] fix: dropdown checkbox click area (#1215) --- src/search-manager/FilterByBlockType.tsx | 21 ++++---- src/search-manager/FilterByTags.tsx | 68 +++++++++++++----------- 2 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/search-manager/FilterByBlockType.tsx b/src/search-manager/FilterByBlockType.tsx index dc65c7ca86..f592b713ae 100644 --- a/src/search-manager/FilterByBlockType.tsx +++ b/src/search-manager/FilterByBlockType.tsx @@ -81,16 +81,17 @@ const FilterByBlockType: React.FC> = () => { { Object.entries(sortedBlockTypes).map(([blockType, count]) => ( - - {' '} - {count} - + )) } { diff --git a/src/search-manager/FilterByTags.tsx b/src/search-manager/FilterByTags.tsx index 476a07ed35..8535ab4485 100644 --- a/src/search-manager/FilterByTags.tsx +++ b/src/search-manager/FilterByTags.tsx @@ -49,38 +49,43 @@ const TagMenuItem: React.FC<{ const randomNumber = React.useMemo(() => Math.floor(Math.random() * 1000), []); const checkboxId = tagPath.replace(/[\W]/g, '_') + randomNumber; + const expandChildrenClick = React.useCallback((e) => { + e.preventDefault(); + onToggleChildren?.(tagPath); + }, [onToggleChildren, tagPath]); + return ( -
- - - { - hasChildren - ? ( - onToggleChildren?.(tagPath)} - variant="primary" - size="sm" - /> - ) : null - } -
+ ); }; @@ -125,7 +130,6 @@ const TagOptions: React.FC<{ return ( Date: Tue, 20 Aug 2024 15:21:54 -0400 Subject: [PATCH 2/8] fix(deps): update dependency @edx/frontend-lib-content-components to v2.6.8 (#1218) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 541a84a616..929fdd915a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2564,9 +2564,9 @@ } }, "node_modules/@edx/frontend-lib-content-components": { - "version": "2.6.6", - "resolved": "https://registry.npmjs.org/@edx/frontend-lib-content-components/-/frontend-lib-content-components-2.6.6.tgz", - "integrity": "sha512-l3jTKgRzxHOO/Xb4w/8el7ZAOQdVS/bl024mgTemPLAB11L4gw/JsK62M6ojp/u9lvGNlaj+GpfzleyLqtJrnQ==", + "version": "2.6.8", + "resolved": "https://registry.npmjs.org/@edx/frontend-lib-content-components/-/frontend-lib-content-components-2.6.8.tgz", + "integrity": "sha512-v1d48qTFEakABOHtfA2mk8Zaxt1PKpsaME4Zyo5wArOAr6ScMtNEY011dJs/PYZAmqXZTmlScsLB8oVxGrbDOA==", "license": "AGPL-3.0", "dependencies": { "@codemirror/lang-html": "^6.0.0", From 3089d0b993fbb55e9a49c85a0d67bec4bd96994b Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 21 Aug 2024 07:42:06 +0530 Subject: [PATCH 3/8] fix: discard button [FC-0062] (#1214) * fix: discard changes * refactor: disable discard btn for new libs Enable it if components are added. * refactor: invalidate library related content queries * chore: add comment about content search query invalidation --- src/library-authoring/data/apiHooks.ts | 20 +++++++++- .../library-info/LibraryInfo.test.tsx | 40 +++++++++++++++++++ .../library-info/LibraryPublishStatus.tsx | 25 +++++------- 3 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 1e9a92bf84..64420a71bb 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -1,4 +1,6 @@ -import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; +import { + useQuery, useMutation, useQueryClient, Query, +} from '@tanstack/react-query'; import { type GetLibrariesV2CustomParams, @@ -122,6 +124,22 @@ export const useRevertLibraryChanges = () => { mutationFn: revertLibraryChanges, onSettled: (_data, _error, libraryId) => { queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); + queryClient.invalidateQueries({ + // Invalidate all content queries related to this library. + // If we allow searching "all courses and libraries" in the future, + // then we'd have to invalidate all `["content_search", "results"]` + // queries, and not just the ones for this library, because items from + // this library could be included in an "all courses and libraries" + // search. For now we only allow searching individual libraries. + predicate: /* istanbul ignore next */ (query: Query): boolean => { + // extraFilter contains library id + const extraFilter = query.queryKey[5]; + if (!(Array.isArray(extraFilter) || typeof extraFilter === 'string')) { + return false; + } + return query.queryKey[0] === 'content_search' && extraFilter?.includes(`context_key = "${libraryId}"`); + }, + }); }, }); }; diff --git a/src/library-authoring/library-info/LibraryInfo.test.tsx b/src/library-authoring/library-info/LibraryInfo.test.tsx index 5ace15eb94..09c8350e08 100644 --- a/src/library-authoring/library-info/LibraryInfo.test.tsx +++ b/src/library-authoring/library-info/LibraryInfo.test.tsx @@ -204,4 +204,44 @@ describe('', () => { await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(url)); }); + + it('should discard changes', async () => { + const url = getCommitLibraryChangesUrl(libraryData.id); + axiosMock.onDelete(url).reply(200); + + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + fireEvent.click(discardButton); + + expect(await screen.findByText('Library changes reverted successfully')).toBeInTheDocument(); + + await waitFor(() => expect(axiosMock.history.delete[0].url).toEqual(url)); + }); + + it('should show error on discard changes', async () => { + const url = getCommitLibraryChangesUrl(libraryData.id); + axiosMock.onDelete(url).reply(500); + + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + fireEvent.click(discardButton); + + expect(await screen.findByText('There was an error reverting changes in the library.')).toBeInTheDocument(); + + await waitFor(() => expect(axiosMock.history.delete[0].url).toEqual(url)); + }); + + it('discard changes btn should be disabled for new libraries', async () => { + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + + expect(discardButton).toBeDisabled(); + }); + + it('discard changes btn should be enabled for new libraries if components are added', async () => { + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + + expect(discardButton).not.toBeDisabled(); + }); }); diff --git a/src/library-authoring/library-info/LibraryPublishStatus.tsx b/src/library-authoring/library-info/LibraryPublishStatus.tsx index e497042657..a45ddccf66 100644 --- a/src/library-authoring/library-info/LibraryPublishStatus.tsx +++ b/src/library-authoring/library-info/LibraryPublishStatus.tsx @@ -2,7 +2,7 @@ import React, { useCallback, useContext, useMemo } from 'react'; import classNames from 'classnames'; import { Button, Container, Stack } from '@openedx/paragon'; import { FormattedDate, FormattedTime, useIntl } from '@edx/frontend-platform/i18n'; -import { useCommitLibraryChanges } from '../data/apiHooks'; +import { useCommitLibraryChanges, useRevertLibraryChanges } from '../data/apiHooks'; import { ContentLibrary } from '../data/api'; import { ToastContext } from '../../generic/toast-context'; import messages from './messages'; @@ -14,6 +14,7 @@ type LibraryPublishStatusProps = { const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { const intl = useIntl(); const commitLibraryChanges = useCommitLibraryChanges(); + const revertLibraryChanges = useRevertLibraryChanges(); const { showToast } = useContext(ToastContext); const commit = useCallback(() => { @@ -25,9 +26,6 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { }); }, []); - /** - * TODO, the discard changes breaks the library. - * Discomment this when discard changes is fixed. const revert = useCallback(() => { revertLibraryChanges.mutateAsync(library.id) .then(() => { @@ -36,15 +34,16 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { showToast(intl.formatMessage(messages.revertErrorMsg)); }); }, []); - */ const { isPublished, + isNew, statusMessage, extraStatusMessage, bodyMessage, } = useMemo(() => { let isPublishedResult: boolean; + let isNewResult = false; let statusMessageResult : string; let extraStatusMessageResult : string | undefined; let bodyMessageResult : string | undefined; @@ -94,6 +93,7 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { if (!library.lastPublished) { // Library is never published (new) + isNewResult = library.numBlocks === 0; // allow discarding if components are added isPublishedResult = false; statusMessageResult = intl.formatMessage(messages.draftStatusLabel); extraStatusMessageResult = intl.formatMessage(messages.neverPublishedLabel); @@ -123,6 +123,7 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { } return { isPublished: isPublishedResult, + isNew: isNewResult, statusMessage: statusMessageResult, extraStatusMessage: extraStatusMessageResult, bodyMessage: bodyMessageResult, @@ -153,15 +154,11 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { - { /* - * TODO, the discard changes breaks the library. - * Discomment this when discard changes is fixed. -
- -
- */ } +
+ +
From 8ae9dfbd8874f87f7d786faebb14a8b892d8497c Mon Sep 17 00:00:00 2001 From: Muhammad Anas <88967643+Anas12091101@users.noreply.github.com> Date: Wed, 21 Aug 2024 19:36:14 +0500 Subject: [PATCH 4/8] feat: customize the certificate link in header (#1223) * feat: customize the certificate link in header * fix: lint issues * fix: tests --- .env | 1 + .env.development | 1 + .env.test | 1 + src/CourseAuthoringRoutes.jsx | 2 +- src/header/utils.js | 57 +++++++++++++++++++---------------- src/header/utils.test.js | 21 ++++++++++++- src/index.jsx | 1 + 7 files changed, 56 insertions(+), 28 deletions(-) diff --git a/.env b/.env index 6f871fb94c..ec000feba0 100644 --- a/.env +++ b/.env @@ -34,6 +34,7 @@ ENABLE_UNIT_PAGE=false ENABLE_ASSETS_PAGE=false ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN=false ENABLE_TAGGING_TAXONOMY_PAGES=true +ENABLE_CERTIFICATE_PAGE=true BBB_LEARN_MORE_URL='' HOTJAR_APP_ID='' HOTJAR_VERSION=6 diff --git a/.env.development b/.env.development index 75b4219d0e..1cc8f24da6 100644 --- a/.env.development +++ b/.env.development @@ -35,6 +35,7 @@ ENABLE_TEAM_TYPE_SETTING=false ENABLE_UNIT_PAGE=false ENABLE_ASSETS_PAGE=false ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN=true +ENABLE_CERTIFICATE_PAGE=true ENABLE_NEW_VIDEO_UPLOAD_PAGE=true ENABLE_TAGGING_TAXONOMY_PAGES=true BBB_LEARN_MORE_URL='' diff --git a/.env.test b/.env.test index 0f73517968..7c591ff68e 100644 --- a/.env.test +++ b/.env.test @@ -31,6 +31,7 @@ ENABLE_TEAM_TYPE_SETTING=false ENABLE_UNIT_PAGE=true ENABLE_ASSETS_PAGE=false ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN=true +ENABLE_CERTIFICATE_PAGE=true ENABLE_TAGGING_TAXONOMY_PAGES=true BBB_LEARN_MORE_URL='' INVITE_STUDENTS_EMAIL_TO="someone@domain.com" diff --git a/src/CourseAuthoringRoutes.jsx b/src/CourseAuthoringRoutes.jsx index 8d8d1a6c06..51599317e6 100644 --- a/src/CourseAuthoringRoutes.jsx +++ b/src/CourseAuthoringRoutes.jsx @@ -124,7 +124,7 @@ const CourseAuthoringRoutes = () => { /> } + element={getConfig().ENABLE_CERTIFICATE_PAGE === 'true' ? : null} /> { return items; }; -export const getSettingMenuItems = ({ studioBaseUrl, courseId, intl }) => ([ - { - href: `${studioBaseUrl}/settings/details/${courseId}`, - title: intl.formatMessage(messages['header.links.scheduleAndDetails']), - }, - { - href: `${studioBaseUrl}/settings/grading/${courseId}`, - title: intl.formatMessage(messages['header.links.grading']), - }, - { - href: `${studioBaseUrl}/course_team/${courseId}`, - title: intl.formatMessage(messages['header.links.courseTeam']), - }, - { - href: `${studioBaseUrl}/group_configurations/${courseId}`, - title: intl.formatMessage(messages['header.links.groupConfigurations']), - }, - { - href: `${studioBaseUrl}/settings/advanced/${courseId}`, - title: intl.formatMessage(messages['header.links.advancedSettings']), - }, - { - href: `${studioBaseUrl}/certificates/${courseId}`, - title: intl.formatMessage(messages['header.links.certificates']), - }, -]); +export const getSettingMenuItems = ({ studioBaseUrl, courseId, intl }) => { + const items = [ + { + href: `${studioBaseUrl}/settings/details/${courseId}`, + title: intl.formatMessage(messages['header.links.scheduleAndDetails']), + }, + { + href: `${studioBaseUrl}/settings/grading/${courseId}`, + title: intl.formatMessage(messages['header.links.grading']), + }, + { + href: `${studioBaseUrl}/course_team/${courseId}`, + title: intl.formatMessage(messages['header.links.courseTeam']), + }, + { + href: `${studioBaseUrl}/group_configurations/${courseId}`, + title: intl.formatMessage(messages['header.links.groupConfigurations']), + }, + { + href: `${studioBaseUrl}/settings/advanced/${courseId}`, + title: intl.formatMessage(messages['header.links.advancedSettings']), + }, + ]; + if (getConfig().ENABLE_CERTIFICATE_PAGE === 'true') { + items.push({ + href: `${studioBaseUrl}/certificates/${courseId}`, + title: intl.formatMessage(messages['header.links.certificates']), + }); + } + return items; +}; export const getToolsMenuItems = ({ studioBaseUrl, courseId, intl }) => ([ { diff --git a/src/header/utils.test.js b/src/header/utils.test.js index afcb5da24d..f2c2f3acb5 100644 --- a/src/header/utils.test.js +++ b/src/header/utils.test.js @@ -1,5 +1,5 @@ import { getConfig, setConfig } from '@edx/frontend-platform'; -import { getContentMenuItems, getToolsMenuItems } from './utils'; +import { getContentMenuItems, getToolsMenuItems, getSettingMenuItems } from './utils'; const props = { studioBaseUrl: 'UrLSTuiO', @@ -29,6 +29,25 @@ describe('header utils', () => { }); }); + describe('getSettingsMenuitems', () => { + it('should include certificates option', () => { + setConfig({ + ...getConfig(), + ENABLE_CERTIFICATE_PAGE: 'true', + }); + const actualItems = getSettingMenuItems(props); + expect(actualItems).toHaveLength(6); + }); + it('should not include certificates option', () => { + setConfig({ + ...getConfig(), + ENABLE_CERTIFICATE_PAGE: 'false', + }); + const actualItems = getSettingMenuItems(props); + expect(actualItems).toHaveLength(5); + }); + }); + describe('getToolsMenuItems', () => { it('should include export tags option', () => { setConfig({ diff --git a/src/index.jsx b/src/index.jsx index 9336b9486f..675e927097 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -126,6 +126,7 @@ initialize({ ENABLE_UNIT_PAGE: process.env.ENABLE_UNIT_PAGE || 'false', ENABLE_ASSETS_PAGE: process.env.ENABLE_ASSETS_PAGE || 'false', ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN: process.env.ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN || 'false', + ENABLE_CERTIFICATE_PAGE: process.env.ENABLE_CERTIFICATE_PAGE || 'false', ENABLE_TAGGING_TAXONOMY_PAGES: process.env.ENABLE_TAGGING_TAXONOMY_PAGES || 'false', ENABLE_HOME_PAGE_COURSE_API_V2: process.env.ENABLE_HOME_PAGE_COURSE_API_V2 === 'true', ENABLE_CHECKLIST_QUALITY: process.env.ENABLE_CHECKLIST_QUALITY || 'true', From 21c9e302073ce93f11157992a82e6b96d27057ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 22 Aug 2024 10:50:43 -0300 Subject: [PATCH 5/8] refactor: change toast component (#1211) --- src/generic/processing-notification/index.jsx | 4 +--- src/generic/toast-context/index.test.tsx | 8 ++++++++ src/generic/toast-context/index.tsx | 15 ++++++++++----- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/generic/processing-notification/index.jsx b/src/generic/processing-notification/index.jsx index 163bf5fd3a..75c718c830 100644 --- a/src/generic/processing-notification/index.jsx +++ b/src/generic/processing-notification/index.jsx @@ -5,8 +5,6 @@ import { Badge, Icon } from '@openedx/paragon'; import { Settings as IconSettings } from '@openedx/paragon/icons'; import { capitalize } from 'lodash'; -import { NOTIFICATION_MESSAGES } from '../../constants'; - const ProcessingNotification = ({ isShow, title }) => ( ( ProcessingNotification.propTypes = { isShow: PropTypes.bool.isRequired, - title: PropTypes.oneOf(Object.values(NOTIFICATION_MESSAGES)).isRequired, + title: PropTypes.string.isRequired, }; export default ProcessingNotification; diff --git a/src/generic/toast-context/index.test.tsx b/src/generic/toast-context/index.test.tsx index ea00ba3a1d..f7e0a2e4b0 100644 --- a/src/generic/toast-context/index.test.tsx +++ b/src/generic/toast-context/index.test.tsx @@ -50,6 +50,7 @@ describe('', () => { }, }); store = initializeStore(); + jest.useFakeTimers(); }); afterEach(() => { @@ -61,6 +62,13 @@ describe('', () => { expect(await screen.findByText('This is the toast!')).toBeInTheDocument(); }); + it('should close toast after 5000ms', async () => { + render(); + expect(await screen.findByText('This is the toast!')).toBeInTheDocument(); + jest.advanceTimersByTime(6000); + expect(screen.queryByText('This is the toast!')).not.toBeInTheDocument(); + }); + it('should close toast', async () => { render(); expect(await screen.findByText('Content')).toBeInTheDocument(); diff --git a/src/generic/toast-context/index.tsx b/src/generic/toast-context/index.tsx index 3e98407844..f4fd2aa332 100644 --- a/src/generic/toast-context/index.tsx +++ b/src/generic/toast-context/index.tsx @@ -1,5 +1,6 @@ import React from 'react'; -import { Toast } from '@openedx/paragon'; + +import ProcessingNotification from '../processing-notification'; export interface ToastContextData { toastMessage: string | null; @@ -35,7 +36,13 @@ export const ToastProvider = (props: ToastProviderProps) => { setToastMessage(null); }, []); - const showToast = React.useCallback((message) => setToastMessage(message), [setToastMessage]); + const showToast = React.useCallback((message) => { + setToastMessage(message); + // Close the toast after 5 seconds + setTimeout(() => { + setToastMessage(null); + }, 5000); + }, [setToastMessage]); const closeToast = React.useCallback(() => setToastMessage(null), [setToastMessage]); const context = React.useMemo(() => ({ @@ -48,9 +55,7 @@ export const ToastProvider = (props: ToastProviderProps) => { {props.children} { toastMessage && ( - - {toastMessage} - + )} ); From 3e0f7b57583ec7f22ba7b492cbd83d327acccf8f Mon Sep 17 00:00:00 2001 From: Bilal Qamar <59555732+BilalQamar95@users.noreply.github.com> Date: Thu, 22 Aug 2024 23:37:36 +0500 Subject: [PATCH 6/8] test: Add Node 20 to CI matrix (#1224) --- .github/workflows/validate.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 7054c4fd37..eab786bce6 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -9,13 +9,16 @@ on: jobs: tests: runs-on: ubuntu-latest + strategy: + matrix: + node: [18, 20] + continue-on-error: ${{ matrix.node == 20 }} + steps: - uses: actions/checkout@v4 - - name: Setup Nodejs Env - run: echo "NODE_VER=`cat .nvmrc`" >> $GITHUB_ENV - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v4 with: - node-version: ${{ env.NODE_VER }} + node-version: ${{ matrix.node }} - run: make validate.ci - name: Upload coverage uses: codecov/codecov-action@v4 From 259a50c468a7e7237eef91e398d6710dc4ea4940 Mon Sep 17 00:00:00 2001 From: Jillian Date: Sat, 24 Aug 2024 09:41:53 +0930 Subject: [PATCH 7/8] UI fixes for Sort Library Component [FC-0059] (#1222) fix: use "Recently Modified" as the default sort option When search keyword(s) are entered, use "Most Relevant" as default. Also * Hides "Most Relevant" option if no keyword is entered. * Re-orders the sort menu options * Ensures the default sort option is not stored in the query string * Shows the selected sort option on the drop-down toggle button * Shows "Sort By" as a header inside the drop-down menu --- .../LibraryAuthoringPage.test.tsx | 111 +++++++++++------- src/search-manager/SearchManager.ts | 22 ++-- src/search-manager/SearchSortWidget.tsx | 65 +++++++--- src/search-manager/messages.ts | 7 +- 4 files changed, 129 insertions(+), 76 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 3dbd737bc5..2481ce80e0 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -189,33 +189,33 @@ describe('', () => { axiosMock.onGet(getContentLibraryApiUrl(libraryData.id)).reply(200, libraryData); const { - getByRole, getByText, queryByText, findByText, findAllByText, + getByRole, getAllByText, getByText, queryByText, findByText, findAllByText, } = render(); - // Ensure the search endpoint is called: - // Call 1: To fetch searchable/filterable/sortable library data - // Call 2: To fetch the recently modified components only - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); expect(await findByText('Content library')).toBeInTheDocument(); expect((await findAllByText(libraryData.title))[0]).toBeInTheDocument(); expect(queryByText('You have not added any content to this library yet.')).not.toBeInTheDocument(); - expect(getByText('Recently Modified')).toBeInTheDocument(); + // "Recently Modified" header + sort shown + expect(getAllByText('Recently Modified').length).toEqual(2); expect(getByText('Collections (0)')).toBeInTheDocument(); expect(getByText('Components (6)')).toBeInTheDocument(); expect((await findAllByText('Test HTML Block'))[0]).toBeInTheDocument(); // Navigate to the components tab fireEvent.click(getByRole('tab', { name: 'Components' })); - expect(queryByText('Recently Modified')).not.toBeInTheDocument(); + // "Recently Modified" default sort shown + expect(getAllByText('Recently Modified').length).toEqual(1); expect(queryByText('Collections (0)')).not.toBeInTheDocument(); expect(queryByText('Components (6)')).not.toBeInTheDocument(); // Navigate to the collections tab fireEvent.click(getByRole('tab', { name: 'Collections' })); - expect(queryByText('Recently Modified')).not.toBeInTheDocument(); + // "Recently Modified" default sort shown + expect(getAllByText('Recently Modified').length).toEqual(1); expect(queryByText('Collections (0)')).not.toBeInTheDocument(); expect(queryByText('Components (6)')).not.toBeInTheDocument(); expect(queryByText('There are 6 components in this library')).not.toBeInTheDocument(); @@ -224,7 +224,8 @@ describe('', () => { // Go back to Home tab // This step is necessary to avoid the url change leak to other tests fireEvent.click(getByRole('tab', { name: 'Home' })); - expect(getByText('Recently Modified')).toBeInTheDocument(); + // "Recently Modified" header + sort shown + expect(getAllByText('Recently Modified').length).toEqual(2); expect(getByText('Collections (0)')).toBeInTheDocument(); expect(getByText('Components (6)')).toBeInTheDocument(); }); @@ -239,10 +240,7 @@ describe('', () => { expect(await findByText('Content library')).toBeInTheDocument(); expect((await findAllByText(libraryData.title))[0]).toBeInTheDocument(); - // Ensure the search endpoint is called: - // Call 1: To fetch searchable/filterable/sortable library data - // Call 2: To fetch the recently modified components only - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); expect(getByText('You have not added any content to this library yet.')).toBeInTheDocument(); }); @@ -304,16 +302,13 @@ describe('', () => { expect(await findByText('Content library')).toBeInTheDocument(); expect((await findAllByText(libraryData.title))[0]).toBeInTheDocument(); - // Ensure the search endpoint is called: - // Call 1: To fetch searchable/filterable/sortable library data - // Call 2: To fetch the recently modified components only - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); fireEvent.change(getByRole('searchbox'), { target: { value: 'noresults' } }); // Ensure the search endpoint is called again, only once more since the recently modified call // should not be impacted by the search - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); expect(getByText('No matching components found in this library.')).toBeInTheDocument(); @@ -396,15 +391,13 @@ describe('', () => { getByRole, getByText, queryByText, getAllByText, findAllByText, } = render(); - // Ensure the search endpoint is called: - // Call 1: To fetch searchable/filterable/sortable library data - // Call 2: To fetch the recently modified components only - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); expect(getByText('Content library')).toBeInTheDocument(); expect((await findAllByText(libraryData.title))[0]).toBeInTheDocument(); - await waitFor(() => { expect(getByText('Recently Modified')).toBeInTheDocument(); }); + // "Recently Modified" header + sort shown + await waitFor(() => { expect(getAllByText('Recently Modified').length).toEqual(2); }); expect(getByText('Collections (0)')).toBeInTheDocument(); expect(getByText('Components (6)')).toBeInTheDocument(); expect(getAllByText('Test HTML Block')[0]).toBeInTheDocument(); @@ -416,7 +409,8 @@ describe('', () => { // Clicking on "View All" button should navigate to the Components tab fireEvent.click(getByText('View All')); - expect(queryByText('Recently Modified')).not.toBeInTheDocument(); + // "Recently Modified" default sort shown + expect(getAllByText('Recently Modified').length).toEqual(1); expect(queryByText('Collections (0)')).not.toBeInTheDocument(); expect(queryByText('Components (6)')).not.toBeInTheDocument(); expect(getAllByText('Test HTML Block')[0]).toBeInTheDocument(); @@ -424,7 +418,8 @@ describe('', () => { // Go back to Home tab // This step is necessary to avoid the url change leak to other tests fireEvent.click(getByRole('tab', { name: 'Home' })); - expect(getByText('Recently Modified')).toBeInTheDocument(); + // "Recently Modified" header + sort shown + expect(getAllByText('Recently Modified').length).toEqual(2); expect(getByText('Collections (0)')).toBeInTheDocument(); expect(getByText('Components (6)')).toBeInTheDocument(); }); @@ -438,15 +433,13 @@ describe('', () => { getByText, queryByText, getAllByText, findAllByText, } = render(); - // Ensure the search endpoint is called: - // Call 1: To fetch searchable/filterable/sortable library data - // Call 2: To fetch the recently modified components only - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); expect(getByText('Content library')).toBeInTheDocument(); expect((await findAllByText(libraryData.title))[0]).toBeInTheDocument(); - await waitFor(() => { expect(getByText('Recently Modified')).toBeInTheDocument(); }); + // "Recently Modified" header + sort shown + await waitFor(() => { expect(getAllByText('Recently Modified').length).toEqual(2); }); expect(getByText('Collections (0)')).toBeInTheDocument(); expect(getByText('Components (2)')).toBeInTheDocument(); expect(getAllByText('Test HTML Block')[0]).toBeInTheDocument(); @@ -463,18 +456,25 @@ describe('', () => { fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); const { - findByTitle, getAllByText, getByText, getByTitle, + findByTitle, getAllByText, getByRole, getByTitle, } = render(); expect(await findByTitle('Sort search results')).toBeInTheDocument(); - const testSortOption = (async (optionText, sortBy) => { - if (optionText) { - fireEvent.click(getByTitle('Sort search results')); - fireEvent.click(getByText(optionText)); - } + const testSortOption = (async (optionText, sortBy, isDefault) => { + // Open the drop-down menu + fireEvent.click(getByTitle('Sort search results')); + + // Click the option with the given text + // Since the sort drop-down also shows the selected sort + // option in its toggle button, we need to make sure we're + // clicking on the last one found. + const options = getAllByText(optionText); + expect(options.length).toBeGreaterThan(0); + fireEvent.click(options[options.length - 1]); + + // Did the search happen with the expected sort option? const bodyText = sortBy ? `"sort":["${sortBy}"]` : '"sort":[]'; - const searchText = sortBy ? `?sort=${encodeURIComponent(sortBy)}` : ''; await waitFor(() => { expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { body: expect.stringContaining(bodyText), @@ -482,16 +482,23 @@ describe('', () => { headers: expect.anything(), }); }); + + // Is the sort option stored in the query string? + const searchText = isDefault ? '' : `?sort=${encodeURIComponent(sortBy)}`; expect(window.location.search).toEqual(searchText); + + // Is the selected sort option shown in the toggle button (if not default) + // as well as in the drop-down menu? + expect(getAllByText(optionText).length).toEqual(isDefault ? 1 : 2); }); - await testSortOption('Title, A-Z', 'display_name:asc'); - await testSortOption('Title, Z-A', 'display_name:desc'); - await testSortOption('Newest', 'created:desc'); - await testSortOption('Oldest', 'created:asc'); + await testSortOption('Title, A-Z', 'display_name:asc', false); + await testSortOption('Title, Z-A', 'display_name:desc', false); + await testSortOption('Newest', 'created:desc', false); + await testSortOption('Oldest', 'created:asc', false); // Sorting by Recently Published also excludes unpublished components - await testSortOption('Recently Published', 'last_published:desc'); + await testSortOption('Recently Published', 'last_published:desc', false); await waitFor(() => { expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { body: expect.stringContaining('last_published IS NOT NULL'), @@ -500,8 +507,22 @@ describe('', () => { }); }); - // Clearing filters clears the url search param and uses default sort - fireEvent.click(getAllByText('Clear Filters')[0]); - await testSortOption('', ''); + // Re-selecting the previous sort option resets sort to default "Recently Modified" + await testSortOption('Recently Published', 'modified:desc', true); + expect(getAllByText('Recently Modified').length).toEqual(2); + + // Enter a keyword into the search box + const searchBox = getByRole('searchbox'); + fireEvent.change(searchBox, { target: { value: 'words to find' } }); + + // Default sort option changes to "Most Relevant" + expect(getAllByText('Most Relevant').length).toEqual(2); + await waitFor(() => { + expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { + body: expect.stringContaining('"sort":[]'), + method: 'POST', + headers: expect.anything(), + }); + }); }); }); diff --git a/src/search-manager/SearchManager.ts b/src/search-manager/SearchManager.ts index 4db9e6f21b..d75a6cbdba 100644 --- a/src/search-manager/SearchManager.ts +++ b/src/search-manager/SearchManager.ts @@ -28,6 +28,7 @@ export interface SearchContextData { isFiltered: boolean; searchSortOrder: SearchSortOption; setSearchSortOrder: React.Dispatch>; + defaultSearchSortOrder: SearchSortOption; hits: ContentHit[]; totalHits: number; isFetching: boolean; @@ -65,14 +66,11 @@ function useStateWithUrlSearchParam( setSearchParams((prevParams) => { const paramValue: string = toString(value) ?? ''; const newSearchParams = new URLSearchParams(prevParams); - if (paramValue) { - newSearchParams.set(paramName, paramValue); - } else { - // If no paramValue, remove it from the search params, so - // we don't get dangling parameter values like ?paramName= - // Another way to decide this would be to check value === defaultValue, - // and ensure that default values are never stored in the search string. + // If using the default paramValue, remove it from the search params. + if (paramValue === defaultValue) { newSearchParams.delete(paramName); + } else { + newSearchParams.set(paramName, paramValue); } return newSearchParams; }, { replace: true }); @@ -95,9 +93,10 @@ export const SearchContextProvider: React.FC<{ // The search sort order can be set via the query string // E.g. ?sort=display_name:desc maps to SearchSortOption.TITLE_ZA. - const defaultSortOption = SearchSortOption.RELEVANCE; + // Default sort by Most Relevant if there's search keyword(s), else by Recently Modified. + const defaultSearchSortOrder = searchKeywords ? SearchSortOption.RELEVANCE : SearchSortOption.RECENTLY_MODIFIED; const [searchSortOrder, setSearchSortOrder] = useStateWithUrlSearchParam( - defaultSortOption, + defaultSearchSortOrder, 'sort', (value: string) => Object.values(SearchSortOption).find((enumValue) => value === enumValue), (value: SearchSortOption) => value.toString(), @@ -105,7 +104,7 @@ export const SearchContextProvider: React.FC<{ // SearchSortOption.RELEVANCE is special, it means "no custom sorting", so we // send it to useContentSearchResults as an empty array. const searchSortOrderToUse = overrideSearchSortOrder ?? searchSortOrder; - const sort: SearchSortOption[] = (searchSortOrderToUse === defaultSortOption ? [] : [searchSortOrderToUse]); + const sort: SearchSortOption[] = (searchSortOrderToUse === SearchSortOption.RELEVANCE ? [] : [searchSortOrderToUse]); // Selecting SearchSortOption.RECENTLY_PUBLISHED also excludes unpublished components. if (searchSortOrderToUse === SearchSortOption.RECENTLY_PUBLISHED) { extraFilter.push('last_published IS NOT NULL'); @@ -114,13 +113,11 @@ export const SearchContextProvider: React.FC<{ const canClearFilters = ( blockTypesFilter.length > 0 || tagsFilter.length > 0 - || searchSortOrderToUse !== defaultSortOption ); const isFiltered = canClearFilters || (searchKeywords !== ''); const clearFilters = React.useCallback(() => { setBlockTypesFilter([]); setTagsFilter([]); - setSearchSortOrder(defaultSortOption); }, []); // Initialize a connection to Meilisearch: @@ -160,6 +157,7 @@ export const SearchContextProvider: React.FC<{ clearFilters, searchSortOrder, setSearchSortOrder, + defaultSearchSortOrder, closeSearchModal: props.closeSearchModal ?? (() => {}), hasError: hasConnectionError || result.isError, ...result, diff --git a/src/search-manager/SearchSortWidget.tsx b/src/search-manager/SearchSortWidget.tsx index 6885859b3c..01309845dd 100644 --- a/src/search-manager/SearchSortWidget.tsx +++ b/src/search-manager/SearchSortWidget.tsx @@ -9,47 +9,71 @@ import { useSearchContext } from './SearchManager'; export const SearchSortWidget: React.FC> = () => { const intl = useIntl(); + const { + searchSortOrder, + setSearchSortOrder, + defaultSearchSortOrder, + } = useSearchContext(); + const menuItems = useMemo( () => [ + { + id: 'search-sort-option-most-relevant', + name: intl.formatMessage(messages.searchSortMostRelevant), + value: SearchSortOption.RELEVANCE, + show: (defaultSearchSortOrder === SearchSortOption.RELEVANCE), + }, + { + id: 'search-sort-option-recently-modified', + name: intl.formatMessage(messages.searchSortRecentlyModified), + value: SearchSortOption.RECENTLY_MODIFIED, + show: true, + }, + { + id: 'search-sort-option-recently-published', + name: intl.formatMessage(messages.searchSortRecentlyPublished), + value: SearchSortOption.RECENTLY_PUBLISHED, + show: true, + }, { id: 'search-sort-option-title-az', name: intl.formatMessage(messages.searchSortTitleAZ), value: SearchSortOption.TITLE_AZ, + show: true, }, { id: 'search-sort-option-title-za', name: intl.formatMessage(messages.searchSortTitleZA), value: SearchSortOption.TITLE_ZA, + show: true, }, { id: 'search-sort-option-newest', name: intl.formatMessage(messages.searchSortNewest), value: SearchSortOption.NEWEST, + show: true, }, { id: 'search-sort-option-oldest', name: intl.formatMessage(messages.searchSortOldest), value: SearchSortOption.OLDEST, - }, - { - id: 'search-sort-option-recently-published', - name: intl.formatMessage(messages.searchSortRecentlyPublished), - value: SearchSortOption.RECENTLY_PUBLISHED, - }, - { - id: 'search-sort-option-recently-modified', - name: intl.formatMessage(messages.searchSortRecentlyModified), - value: SearchSortOption.RECENTLY_MODIFIED, + show: true, }, ], - [intl], + [intl, defaultSearchSortOrder], ); - const { searchSortOrder, setSearchSortOrder } = useSearchContext(); - const selectedSortOption = menuItems.find((menuItem) => menuItem.value === searchSortOrder); - const searchSortLabel = ( - selectedSortOption ? selectedSortOption.name : intl.formatMessage(messages.searchSortWidgetLabel) + const menuHeader = intl.formatMessage(messages.searchSortWidgetLabel); + const defaultSortOption = menuItems.find( + ({ value }) => (value === defaultSearchSortOrder), ); + const shownMenuItems = menuItems.filter(({ show }) => show); + + // Show the currently selected sort option as the toggle button label. + const selectedSortOption = shownMenuItems.find( + ({ value }) => (value === searchSortOrder), + ) ?? defaultSortOption; + const toggleLabel = selectedSortOption ? selectedSortOption.name : menuHeader; return ( @@ -62,13 +86,18 @@ export const SearchSortWidget: React.FC> = () => { size="sm" > - {searchSortLabel} +
{toggleLabel}
- {menuItems.map(({ id, name, value }) => ( + {menuHeader} + {shownMenuItems.map(({ id, name, value }) => ( setSearchSortOrder(value)} + onClick={() => { + // If the selected sort option was re-clicked, de-select it (reset to default) + const searchOrder = value === searchSortOrder ? defaultSearchSortOrder : value; + setSearchSortOrder(searchOrder); + }} > {name} {(value === searchSortOrder) && } diff --git a/src/search-manager/messages.ts b/src/search-manager/messages.ts index 8cd2e506ef..1fa3e229ea 100644 --- a/src/search-manager/messages.ts +++ b/src/search-manager/messages.ts @@ -132,7 +132,7 @@ const messages = defineMessages({ }, searchSortWidgetLabel: { id: 'course-authoring.course-search.searchSortWidget.label', - defaultMessage: 'Sort', + defaultMessage: 'Sort By', description: 'Label displayed to users when default sorting is used by the content search drop-down menu', }, searchSortWidgetAltTitle: { @@ -170,6 +170,11 @@ const messages = defineMessages({ defaultMessage: 'Recently Modified', description: 'Label for the content search sort drop-down which sorts by modified date, descending', }, + searchSortMostRelevant: { + id: 'course-authoring.course-search.searchSort.mostRelevant', + defaultMessage: 'Most Relevant', + description: 'Label for the content search sort drop-down which sorts keyword searches by relevance', + }, }); export default messages; From d99a09efba39aa8e70a44b910bf9a95e4ef2e1e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Mon, 26 Aug 2024 15:33:54 -0500 Subject: [PATCH 8/8] fix: Library v2 info sidebar UI fixes (#1226) --- .../LibraryAuthoringPage.scss | 11 +++++++ .../LibraryAuthoringPage.test.tsx | 9 ++++-- .../LibraryAuthoringPage.tsx | 30 +++++++++++++++---- src/library-authoring/index.scss | 1 + .../library-info/LibraryInfoHeader.tsx | 4 ++- .../library-sidebar/LibrarySidebar.tsx | 2 ++ 6 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 src/library-authoring/LibraryAuthoringPage.scss diff --git a/src/library-authoring/LibraryAuthoringPage.scss b/src/library-authoring/LibraryAuthoringPage.scss new file mode 100644 index 0000000000..3ce8cb718a --- /dev/null +++ b/src/library-authoring/LibraryAuthoringPage.scss @@ -0,0 +1,11 @@ +.library-authoring-page { + .header-actions { + .normal-border { + border: 1px solid; + } + + .open-border { + border: 2px solid; + } + } +} diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 2481ce80e0..b2fe866a0e 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -370,17 +370,22 @@ describe('', () => { expect((await screen.findAllByText(libraryData.title))[0]).toBeInTheDocument(); expect((await screen.findAllByText(libraryData.title))[1]).toBeInTheDocument(); + // Open by default; close the library info sidebar const closeButton = screen.getByRole('button', { name: /close/i }); fireEvent.click(closeButton); - expect(screen.queryByText('Draft')).not.toBeInTheDocument(); expect(screen.queryByText('(Never Published)')).not.toBeInTheDocument(); + // Open library info sidebar with 'Library info' button const libraryInfoButton = screen.getByRole('button', { name: /library info/i }); fireEvent.click(libraryInfoButton); - expect(screen.getByText('Draft')).toBeInTheDocument(); expect(screen.getByText('(Never Published)')).toBeInTheDocument(); + + // CLose library info sidebar with 'Library info' button + fireEvent.click(libraryInfoButton); + expect(screen.queryByText('Draft')).not.toBeInTheDocument(); + expect(screen.queryByText('(Never Published)')).not.toBeInTheDocument(); }); it('show the "View All" button when viewing library with many components', async () => { diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index c2eb969292..c2cc8bc7f9 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -1,4 +1,5 @@ import React, { useContext, useEffect } from 'react'; +import classNames from 'classnames'; import { StudioFooter } from '@edx/frontend-component-footer'; import { useIntl } from '@edx/frontend-platform/i18n'; import { @@ -33,7 +34,7 @@ import LibraryCollections from './LibraryCollections'; import LibraryHome from './LibraryHome'; import { useContentLibrary } from './data/apiHooks'; import { LibrarySidebar } from './library-sidebar'; -import { LibraryContext } from './common/context'; +import { LibraryContext, SidebarBodyComponentId } from './common/context'; import messages from './messages'; enum TabList { @@ -51,22 +52,41 @@ const HeaderActions = ({ canEditLibrary }: HeaderActionsProps) => { const { openAddContentSidebar, openInfoSidebar, + closeLibrarySidebar, + sidebarBodyComponent, } = useContext(LibraryContext); if (!canEditLibrary) { return null; } + const infoSidebarIsOpen = () => ( + sidebarBodyComponent === SidebarBodyComponentId.Info + ); + + const handleOnClickInfoSidebar = () => { + if (infoSidebarIsOpen()) { + closeLibrarySidebar(); + } else { + openInfoSidebar(); + } + }; + return ( - <> +
- +
); }; @@ -132,7 +152,7 @@ const LibraryAuthoringPage = () => { }; return ( - +
{ ) : ( <> - + {library.title} {library.canEditLibrary && ( @@ -75,6 +75,8 @@ const LibraryInfoHeader = ({ library } : LibraryInfoHeaderProps) => { iconAs={Icon} alt={intl.formatMessage(messages.editNameButtonAlt)} onClick={handleClick} + className="mt-1" + size="inline" /> )} diff --git a/src/library-authoring/library-sidebar/LibrarySidebar.tsx b/src/library-authoring/library-sidebar/LibrarySidebar.tsx index 314de8792a..1f8afe782f 100644 --- a/src/library-authoring/library-sidebar/LibrarySidebar.tsx +++ b/src/library-authoring/library-sidebar/LibrarySidebar.tsx @@ -49,11 +49,13 @@ const LibrarySidebar = ({ library }: LibrarySidebarProps) => { {buildHeader()}