feat: Search a11y updated behavior. (#1614)

* feat: Updated accesibility on courseware search modal

* chore: Updated i18n to use useIntl() hook and dialog behavior updates

* feat: Added close when clicking on the modal backdrop

* chore: Swapped remove listeners with an AbortController

* fix: Fixed tests

* chore: Rolled back unintended husky script change

Author: rijuma

src/course-home/courseware-search/CoursewareSearch.jsx

@@ -1,8 +1,8 @@
-import React, { useEffect } from 'react';
+import React, { useEffect, useRef } from 'react';
 import { useParams } from 'react-router';
 import { useDispatch } from 'react-redux';
 import { sendTrackingLogEvent } from '@edx/frontend-platform/analytics';
-import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import { useIntl } from '@edx/frontend-platform/i18n';
 import {
   Alert, Button, Icon, Spinner,
 } from '@openedx/paragon';
@@ -18,7 +18,8 @@ import CoursewareSearchResultsFilterContainer from './CoursewareResultsFilter';
 import { updateModel, useModel } from '../../generic/model-store';
 import { searchCourseContent } from '../data/thunks';
 
-const CoursewareSearch = ({ intl, ...sectionProps }) => {
+const CoursewareSearch = ({ ...sectionProps }) => {
+  const { formatMessage } = useIntl();
   const { courseId } = useParams();
   const { query: searchKeyword, setQuery, clearSearchParams } = useCoursewareSearchParams();
   const dispatch = useDispatch();
@@ -29,6 +30,7 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
     errors,
     total,
   } = useModel('contentSearchResults', courseId);
+  const dialogRef = useRef();
 
   useLockScroll();
 
@@ -44,7 +46,8 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
         searchKeyword: '',
         results: [],
         errors: undefined,
-        loading: false,
+        loading:
+        false,
       },
     }));
   };
@@ -66,20 +69,46 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
     setQuery(value);
   };
 
-  useEffect(() => {
-    handleSubmit(searchKeyword);
-  }, []);
-
   const handleOnChange = (value) => {
     if (value === searchKeyword) { return; }
     if (!value) { clearSearch(); }
   };
 
-  const handleSearchCloseClick = () => {
+  const close = () => {
     clearSearch();
     dispatch(setShowSearch(false));
   };
 
+  const handlePopState = () => close();
+
+  const handleBackdropClick = function (event) {
+    if (event.target === dialogRef.current) {
+      dialogRef.current.close();
+    }
+  };
+
+  useEffect(() => {
+    // We need this to keep the dialog reference when unmounting.
+    const dialog = dialogRef.current;
+
+    // Open the dialog as a modal on render to confine focus within it.
+    dialogRef.current.showModal();
+
+    if (searchKeyword) {
+      handleSubmit(searchKeyword); // In case it's opened with a search link, we run the search.
+    }
+
+    const controller = new AbortController();
+    const { signal } = controller;
+
+    window.addEventListener('popstate', handlePopState, { signal });
+    dialog.addEventListener('click', handleBackdropClick, { signal });
+
+    return () => controller.abort(); // Removes event listeners.
+  }, []);
+
+  const handleSearchClose = () => close();
+
   let status = 'idle';
   if (loading) {
     status = 'loading';
@@ -90,34 +119,37 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
   }
 
   return (
-    <section className="courseware-search" style={{ '--modal-top-position': top }} data-testid="courseware-search-section" {...sectionProps}>
-      <div className="courseware-search__close">
-        <Button
-          variant="tertiary"
-          className="p-1"
-          aria-label={intl.formatMessage(messages.searchCloseAction)}
-          onClick={handleSearchCloseClick}
-          data-testid="courseware-search-close-button"
-        ><Icon src={Close} />
-        </Button>
-      </div>
+    <dialog ref={dialogRef} className="courseware-search" style={{ '--modal-top-position': top }} data-testid="courseware-search-dialog" onClose={handleSearchClose} {...sectionProps}>
       <div className="courseware-search__outer-content">
-        <div className="courseware-search__content">
-          <h1 className="h2">{intl.formatMessage(messages.searchModuleTitle)}</h1>
-          <CoursewareSearchForm
-            searchTerm={searchKeyword}
-            onSubmit={handleSubmit}
-            onChange={handleOnChange}
-            placeholder={intl.formatMessage(messages.searchBarPlaceholderText)}
-          />
+        <div className="courseware-search__content" data-testid="courseware-search-content">
+          <div className="courseware-search__form">
+            <h1 className="h2">{formatMessage(messages.searchModuleTitle)}</h1>
+            <CoursewareSearchForm
+              searchTerm={searchKeyword}
+              onSubmit={handleSubmit}
+              onChange={handleOnChange}
+              placeholder={formatMessage(messages.searchBarPlaceholderText)}
+            />
+            <div className="courseware-search__close">
+              <Button
+                variant="tertiary"
+                className="p-1"
+                aria-label={formatMessage(messages.searchCloseAction)}
+                onClick={() => dialogRef.current.close()}
+                data-testid="courseware-search-close-button"
+              ><Icon src={Close} />
+              </Button>
+            </div>
+          </div>
+
           {status === 'loading' ? (
             <div className="courseware-search__spinner" data-testid="courseware-search-spinner">
-              <Spinner animation="border" variant="light" screenReaderText={intl.formatMessage(messages.loading)} />
+              <Spinner animation="border" variant="light" screenReaderText={formatMessage(messages.loading)} />
             </div>
           ) : null}
           {status === 'error' && (
             <Alert className="mt-4" variant="danger" data-testid="courseware-search-error">
-              {intl.formatMessage(messages.searchResultsError)}
+              {formatMessage(messages.searchResultsError)}
             </Alert>
           )}
           {status === 'results' ? (
@@ -129,20 +161,16 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
                   aria-relevant="all"
                   aria-atomic="true"
                   data-testid="courseware-search-summary"
-                >{intl.formatMessage(messages.searchResultsLabel, { total, keyword: lastSearchKeyword })}
+                >{formatMessage(messages.searchResultsLabel, { total, keyword: lastSearchKeyword })}
                 </div>
               ) : null}
               <CoursewareSearchResultsFilterContainer />
             </>
           ) : null}
         </div>
       </div>
-    </section>
+    </dialog>
   );
 };
 
-CoursewareSearch.propTypes = {
-  intl: intlShape.isRequired,
-};
-
-export default injectIntl(CoursewareSearch);
+export default CoursewareSearch;

src/course-home/courseware-search/CoursewareSearch.test.jsx

@@ -19,6 +19,7 @@ import { updateModel, useModel } from '../../generic/model-store';
 
 jest.mock('./hooks');
 jest.mock('../../generic/model-store', () => ({
+  ...jest.requireActual('../../generic/model-store'),
   updateModel: jest.fn(),
   useModel: jest.fn(),
 }));
@@ -56,7 +57,7 @@ const defaultProps = {
   total: 0,
 };
 
-const coursewareSearch = {
+const defaultSearchParams = {
   query: '',
   filter: '',
   setQuery: jest.fn(),
@@ -96,14 +97,20 @@ const mockModels = ((props = defaultProps) => {
   });
 });
 
-const mockSearchParams = ((props = coursewareSearch) => {
+const mockSearchParams = ((params) => {
+  const props = { ...defaultSearchParams, ...params };
   useCoursewareSearchParams.mockReturnValue(props);
 });
 
 describe('CoursewareSearch', () => {
-  beforeAll(initializeMockApp);
+  beforeAll(() => initializeMockApp());
 
   beforeEach(() => {
+    mockModels();
+    mockSearchParams();
+  });
+
+  afterEach(() => {
     jest.clearAllMocks();
   });
 
@@ -113,65 +120,56 @@ describe('CoursewareSearch', () => {
     });
 
     it('should use useElementBoundingBox() and useLockScroll() hooks', () => {
-      mockModels();
-      mockSearchParams();
       renderComponent();
 
-      expect(useElementBoundingBox).toBeCalledTimes(1);
-      expect(useLockScroll).toBeCalledTimes(1);
+      expect(useElementBoundingBox).toHaveBeenCalledTimes(1);
+      expect(useLockScroll).toHaveBeenCalledTimes(1);
     });
 
     it('should have a "--modal-top-position" CSS variable matching the CourseTabsNavigation top position', () => {
-      mockModels();
-      mockSearchParams();
       renderComponent();
 
-      const section = screen.getByTestId('courseware-search-section');
+      const section = screen.getByTestId('courseware-search-dialog');
       expect(section.style.getPropertyValue('--modal-top-position')).toBe(`${tabsTopPosition}px`);
     });
   });
 
   describe('when clicking on the "Close" button', () => {
-    it('should dispatch setShowSearch(false)', async () => {
-      mockModels();
+    it('should close the dialog', async () => {
       renderComponent();
 
       await waitFor(() => {
         const close = screen.queryByTestId('courseware-search-close-button');
         fireEvent.click(close);
       });
 
-      expect(setShowSearch).toBeCalledWith(false);
+      expect(HTMLDialogElement.prototype.close).toHaveBeenCalled();
+      expect(setShowSearch).toHaveBeenCalledWith(false);
     });
   });
 
   describe('when CourseTabsNavigation is not present', () => {
     it('should use "--modal-top-position: 0" if  nce element is not present', () => {
       useElementBoundingBox.mockImplementation(() => undefined);
 
-      mockModels();
-      mockSearchParams();
       renderComponent();
 
-      const section = screen.getByTestId('courseware-search-section');
+      const section = screen.getByTestId('courseware-search-dialog');
       expect(section.style.getPropertyValue('--modal-top-position')).toBe('0');
     });
   });
 
   describe('when passing extra props', () => {
     it('should pass on extra props to section element', () => {
-      mockModels();
-      mockSearchParams();
       renderComponent({ foo: 'bar' });
 
-      const section = screen.getByTestId('courseware-search-section');
+      const section = screen.getByTestId('courseware-search-dialog');
       expect(section).toHaveAttribute('foo', 'bar');
     });
   });
 
   describe('when submitting an empty search', () => {
     it('should clear the search by dispatch updateModel', async () => {
-      mockModels();
       renderComponent();
 
       await waitFor(() => {
@@ -203,7 +201,6 @@ describe('CoursewareSearch', () => {
     });
 
     it('should call searchCourseContent', async () => {
-      mockModels();
       renderComponent();
 
       const searchKeyword = 'course';
@@ -259,6 +256,7 @@ describe('CoursewareSearch', () => {
 
   describe('when clearing the search input', () => {
     it('should clear the search by dispatch updateModel', async () => {
+      mockSearchParams({ query: 'fubar' });
       mockModels({
         searchKeyword: 'fubar',
         total: 2,

src/course-home/courseware-search/CoursewareSearchForm.jsx

@@ -1,43 +1,44 @@
-import React from 'react';
 import PropTypes from 'prop-types';
 import { SearchField } from '@openedx/paragon';
-import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import { useIntl } from '@edx/frontend-platform/i18n';
 import messages from './messages';
 
 const CoursewareSearchForm = ({
-  intl,
   searchTerm,
   onSubmit,
   onChange,
   placeholder,
-}) => (
-  <SearchField.Advanced
-    value={searchTerm}
-    onSubmit={onSubmit}
-    onChange={onChange}
-    submitButtonLocation="external"
-    className="courseware-search-form"
-    screenReaderText={{
-      label: intl.formatMessage(messages.searchSubmitLabel),
-      clearButton: intl.formatMessage(messages.searchClearAction),
-      submitButton: null, // Remove the sr-only label in the button.
-    }}
-  >
-    <div className="pgn__searchfield_wrapper" data-testid="courseware-search-form">
-      <SearchField.Label />
-      <SearchField.Input placeholder={placeholder} autoFocus />
-      <SearchField.ClearButton />
-    </div>
-    <SearchField.SubmitButton
-      buttonText={intl.formatMessage(messages.searchSubmitLabel)}
+}) => {
+  const { formatMessage } = useIntl();
+
+  return (
+    <SearchField.Advanced
+      value={searchTerm}
+      onSubmit={onSubmit}
+      onChange={onChange}
       submitButtonLocation="external"
-      data-testid="courseware-search-form-submit"
-    />
-  </SearchField.Advanced>
-);
+      className="courseware-search-form"
+      screenReaderText={{
+        label: formatMessage(messages.searchSubmitLabel),
+        clearButton: formatMessage(messages.searchClearAction),
+        submitButton: null, // Remove the sr-only label in the button.
+      }}
+    >
+      <div className="pgn__searchfield_wrapper" data-testid="courseware-search-form">
+        <SearchField.Label />
+        <SearchField.Input placeholder={placeholder} autoFocus />
+        <SearchField.ClearButton />
+      </div>
+      <SearchField.SubmitButton
+        buttonText={formatMessage(messages.searchSubmitLabel)}
+        submitButtonLocation="external"
+        data-testid="courseware-search-form-submit"
+      />
+    </SearchField.Advanced>
+  );
+};
 
 CoursewareSearchForm.propTypes = {
-  intl: intlShape.isRequired,
   searchTerm: PropTypes.string,
   onSubmit: PropTypes.func,
   onChange: PropTypes.func,
@@ -51,4 +52,4 @@ CoursewareSearchForm.defaultProps = {
   placeholder: undefined,
 };
 
-export default injectIntl(CoursewareSearchForm);
+export default CoursewareSearchForm;