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 `<LibraryLayout/>` 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.
This commit is contained in:
@@ -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(() => {
|
||||
|
||||
@@ -111,6 +111,10 @@ describe('<LibraryAuthoringPage />', () => {
|
||||
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
|
||||
|
||||
@@ -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) => (
|
||||
<LibraryProvider
|
||||
/** We need to pass the pathname as key to the LibraryProvider to force a
|
||||
* re-render when we navigate to a new path or page. */
|
||||
key={location.pathname}
|
||||
/** We need to pass the collectionId as key to the LibraryProvider to force a re-render
|
||||
* when we navigate to a collection page. */
|
||||
key={collectionId}
|
||||
libraryId={libraryId}
|
||||
/** The component picker modal to use. We need to pass it as a reference instead of
|
||||
* directly importing it to avoid the import cycle:
|
||||
@@ -44,7 +48,7 @@ const LibraryLayout = () => {
|
||||
</>
|
||||
</SidebarProvider>
|
||||
</LibraryProvider>
|
||||
), [location.pathname]);
|
||||
), [collectionId]);
|
||||
|
||||
return (
|
||||
<Routes>
|
||||
|
||||
@@ -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 }
|
||||
>
|
||||
<SearchField.Label />
|
||||
<SearchField.Input
|
||||
autoFocus
|
||||
autoFocus={Boolean(props.autoFocus)}
|
||||
placeholder={placeholder}
|
||||
/>
|
||||
<SearchField.ClearButton />
|
||||
|
||||
@@ -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('<SearchModal />', () => {
|
||||
const { findByText } = render(<RootWrapper />);
|
||||
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(<RootWrapper />);
|
||||
expect(screen.getByRole('searchbox')).toHaveFocus();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -21,7 +21,11 @@ const SearchModal: React.FC<{ courseId?: string, isOpen: boolean, onClose: () =>
|
||||
isFullscreenOnMobile
|
||||
className="courseware-search-modal"
|
||||
>
|
||||
<SearchUI courseId={courseId} closeSearchModal={props.onClose} />
|
||||
<SearchUI
|
||||
courseId={courseId}
|
||||
closeSearchModal={props.onClose}
|
||||
autoFocus
|
||||
/>
|
||||
</ModalDialog>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -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 }> =
|
||||
<ModalDialog.Header style={{ zIndex: 9 }} className="border-bottom">
|
||||
<ModalDialog.Title><FormattedMessage {...messages.title} /></ModalDialog.Title>
|
||||
<div className="d-flex mt-3">
|
||||
<SearchKeywordsField className="flex-grow-1 mr-2" />
|
||||
<SearchKeywordsField
|
||||
className="flex-grow-1 mr-2"
|
||||
autoFocus={props.autoFocus}
|
||||
/>
|
||||
<SelectMenu variant="primary">
|
||||
<MenuItem
|
||||
onClick={switchToThisCourse}
|
||||
|
||||
Reference in New Issue
Block a user