From 654c06b5961331bd4dae5a67eab85062ab035063 Mon Sep 17 00:00:00 2001 From: Jillian Date: Thu, 23 Jan 2025 03:09:47 +1030 Subject: [PATCH] fix: a11y and performance issues with library authoring navigation [FC-0076] (#1593) Fixes some a11y and performance issues for Library Authors **Accessibility** * When navigating between the various Library Authoring pages, the search keyword box should not be auto-focused, so focus will follow the user's click/tab events and keep that continuity. The course content search modal behavior is unchanged -- loading this modal auto-focuses the search input box as one would expect. **Performance** * Navigating between the various Library Authoring pages was causing a full re-mount of the `` component * React query's `staleTime` option is used to control how long data is considered fresh in both queries and routes. By default, the `staleTime` for queries is set to 0 milliseconds, meaning that the data will always be considered stale and will be refetched whenever the query is mounted. --- src/index.jsx | 8 +++++++- .../LibraryAuthoringPage.test.tsx | 4 ++++ src/library-authoring/LibraryLayout.tsx | 18 +++++++++++------- src/search-manager/SearchKeywordsField.tsx | 8 ++++++-- src/search-modal/SearchModal.test.tsx | 12 +++++++++++- src/search-modal/SearchModal.tsx | 6 +++++- src/search-modal/SearchUI.tsx | 11 +++++++++-- 7 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/index.jsx b/src/index.jsx index 34f27f1b9a..c885dac167 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -35,7 +35,13 @@ import { ToastProvider } from './generic/toast-context'; import 'react-datepicker/dist/react-datepicker.css'; import './index.scss'; -const queryClient = new QueryClient(); +const queryClient = new QueryClient({ + defaultOptions: { + queries: { + staleTime: 60 * 60_000, // If cache is up to one hour old, no need to re-fetch + }, + }, +}); const App = () => { useEffect(() => { diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 2b55375fd2..ed02d68bd1 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -111,6 +111,10 @@ describe('', () => { expect(screen.getAllByText('Recently Modified').length).toEqual(1); expect((await screen.findAllByText('Introduction to Testing'))[0]).toBeInTheDocument(); + // Search box should not have focus on page load + const searchBox = screen.getByRole('searchbox'); + expect(searchBox).not.toHaveFocus(); + // Navigate to the components tab fireEvent.click(screen.getByRole('tab', { name: 'Components' })); // "Recently Modified" default sort shown diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index c093af7ad3..add33f95a0 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -2,11 +2,12 @@ import { useCallback } from 'react'; import { Route, Routes, + useMatch, useParams, - useLocation, + type PathMatch, } from 'react-router-dom'; -import { ROUTES } from './routes'; +import { BASE_ROUTE, ROUTES } from './routes'; import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context/LibraryContext'; import { SidebarProvider } from './common/context/SidebarContext'; @@ -23,12 +24,15 @@ const LibraryLayout = () => { throw new Error('Error: route is missing libraryId.'); } - const location = useLocation(); + // The top-level route is `${BASE_ROUTE}/*`, so match will always be non-null. + const match = useMatch(`${BASE_ROUTE}${ROUTES.COLLECTION}`) as PathMatch<'libraryId' | 'collectionId'> | null; + const collectionId = match?.params.collectionId; + const context = useCallback((childPage) => ( { - ), [location.pathname]); + ), [collectionId]); return ( diff --git a/src/search-manager/SearchKeywordsField.tsx b/src/search-manager/SearchKeywordsField.tsx index 14a6a06dc9..90c09fdd93 100644 --- a/src/search-manager/SearchKeywordsField.tsx +++ b/src/search-manager/SearchKeywordsField.tsx @@ -7,7 +7,11 @@ import { useSearchContext } from './SearchManager'; /** * The "main" input field where users type in search keywords. The search happens as they type (no need to press enter). */ -const SearchKeywordsField: React.FC<{ className?: string, placeholder?: string }> = (props) => { +const SearchKeywordsField: React.FC<{ + className?: string, + placeholder?: string, + autoFocus?: boolean, +}> = (props) => { const intl = useIntl(); const { searchKeywords, setSearchKeywords, usageKey } = useSearchContext(); const defaultPlaceholder = usageKey ? messages.clearUsageKeyToSearch : messages.inputPlaceholder; @@ -24,7 +28,7 @@ const SearchKeywordsField: React.FC<{ className?: string, placeholder?: string } > diff --git a/src/search-modal/SearchModal.test.tsx b/src/search-modal/SearchModal.test.tsx index ef35726395..c7884fbf8c 100644 --- a/src/search-modal/SearchModal.test.tsx +++ b/src/search-modal/SearchModal.test.tsx @@ -5,7 +5,7 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { AppProvider } from '@edx/frontend-platform/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { render } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import type { Store } from 'redux'; import MockAdapter from 'axios-mock-adapter'; @@ -73,4 +73,14 @@ describe('', () => { const { findByText } = render(); expect(await findByText('An error occurred. Unable to load search results.')).toBeInTheDocument(); }); + + it('should set focus on the search input box when loaded in the modal', async () => { + axiosMock.onGet(getContentSearchConfigUrl()).replyOnce(200, { + url: 'https://meilisearch.example.com', + index: 'test-index', + apiKey: 'test-api-key', + }); + render(); + expect(screen.getByRole('searchbox')).toHaveFocus(); + }); }); diff --git a/src/search-modal/SearchModal.tsx b/src/search-modal/SearchModal.tsx index 2e552fb6e8..197ba6c708 100644 --- a/src/search-modal/SearchModal.tsx +++ b/src/search-modal/SearchModal.tsx @@ -21,7 +21,11 @@ const SearchModal: React.FC<{ courseId?: string, isOpen: boolean, onClose: () => isFullscreenOnMobile className="courseware-search-modal" > - + ); }; diff --git a/src/search-modal/SearchUI.tsx b/src/search-modal/SearchUI.tsx index 2df074111c..a70e1f69fe 100644 --- a/src/search-modal/SearchUI.tsx +++ b/src/search-modal/SearchUI.tsx @@ -19,7 +19,11 @@ import EmptyStates from './EmptyStates'; import SearchResults from './SearchResults'; import messages from './messages'; -const SearchUI: React.FC<{ courseId?: string, closeSearchModal?: () => void }> = (props) => { +const SearchUI: React.FC<{ + courseId?: string, + autoFocus?: boolean, + closeSearchModal?: () => void, +}> = (props) => { const hasCourseId = Boolean(props.courseId); const [searchThisCourseEnabled, setSearchThisCourse] = React.useState(hasCourseId); const switchToThisCourse = React.useCallback(() => setSearchThisCourse(true), []); @@ -39,7 +43,10 @@ const SearchUI: React.FC<{ courseId?: string, closeSearchModal?: () => void }> =
- +