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
2025-02-28 12:12:39 -03:00
parent ec360bc545
commit db3f1b9cb0
6 changed files with 151 additions and 93 deletions
--- a/src/course-home/courseware-search/CoursewareSearch.jsx
+++ b/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,7 +161,7 @@ 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 />
@@ -137,12 +169,8 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
          ) : null}
        </div>
      </div>
-    </section>
+    </dialog>
  );
 };

-CoursewareSearch.propTypes = {
-  intl: intlShape.isRequired,
-};
-
-export default injectIntl(CoursewareSearch);
+export default CoursewareSearch;
--- a/src/course-home/courseware-search/CoursewareSearch.test.jsx
+++ b/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,27 +120,22 @@ 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(() => {
@@ -141,7 +143,8 @@ describe('CoursewareSearch', () => {
        fireEvent.click(close);
      });

-      expect(setShowSearch).toBeCalledWith(false);
+      expect(HTMLDialogElement.prototype.close).toHaveBeenCalled();
+      expect(setShowSearch).toHaveBeenCalledWith(false);
    });
  });

@@ -149,29 +152,24 @@ describe('CoursewareSearch', () => {
    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,
--- a/src/course-home/courseware-search/CoursewareSearchForm.jsx
+++ b/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;
--- a/src/course-home/courseware-search/courseware-search.scss
+++ b/src/course-home/courseware-search/courseware-search.scss
@@ -5,13 +5,25 @@
  left: 0;
  right: 0;
  bottom: 0;
+  width: 100%;
+  height: 100%;
+  max-width: none;
+  margin: 0;
  border-top: 1px solid $light-300;
  z-index: $zindex-modal; // Bootstrap's z-index layer for Modals.

+  &__form {
+    position: relative;
+
+    .h2 {
+      margin-right: 2.5rem;
+    }
+  }
+
  &__close {
    position: absolute !important; // For some reason it gets overridden
-    top: 0.5rem;
-    right: 1rem;
+    top: 0;
+    right: 0;
    font-size: 1.5rem;
    line-height: 1;
  }
@@ -152,9 +164,16 @@
 }

@media (min-width: map-get($grid-breakpoints, 'md')) {
-  .courseware-search__content {
-    padding-top: 8rem;
+  .courseware-search {
+    &__close {
+      right: -2.5rem;
+    }
+
+    &__content {
+      padding-top: 8rem;
+    }
  }
+
 }

 body._search-no-scroll {
--- a/src/course-tabs/CourseTabsNavigation.test.jsx
+++ b/src/course-tabs/CourseTabsNavigation.test.jsx
@@ -73,13 +73,13 @@ describe('Course Tabs Navigation', () => {
  it('should NOT render CoursewareSearch if the flag is off', () => {
    renderComponent();

-    expect(screen.queryByTestId('courseware-search-section')).not.toBeInTheDocument();
+    expect(screen.queryByTestId('courseware-search-dialog')).not.toBeInTheDocument();
  });

  it('should render CoursewareSearch if the flag is on', () => {
    useCoursewareSearchState.mockImplementation(() => ({ show: true }));
    renderComponent();

-    expect(screen.queryByTestId('courseware-search-section')).toBeInTheDocument();
+    expect(screen.queryByTestId('courseware-search-dialog')).toBeInTheDocument();
  });
 });
--- a/src/setupTest.js
+++ b/src/setupTest.js
@@ -61,6 +61,18 @@ const supressWarningBlock = (callback) => {
 };
 /* eslint-enable no-console */

+// Mocks for HTML Dialogs behavior. */
+// jsdom does not support HTML Dialogs yet: https://github.com/jsdom/jsdom/issues/3294
+HTMLDialogElement.prototype.show = jest.fn();
+HTMLDialogElement.prototype.showModal = jest.fn(function mock() {
+  const onShowModal = new CustomEvent('show_modal');
+  this.dispatchEvent(onShowModal);
+});
+HTMLDialogElement.prototype.close = jest.fn(function mock() {
+  const onClose = new CustomEvent('close');
+  this.dispatchEvent(onClose);
+});
+
 // Mock Intersection Observer which is unavailable in the context of a test.
 global.IntersectionObserver = jest.fn(function mockIntersectionObserver() {
  this.observe = jest.fn();