From 9dc45e192da0fda8e45b058aad3643ef92dd2d1a Mon Sep 17 00:00:00 2001
From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com>
Date: Fri, 31 Jan 2025 14:18:28 -0500
Subject: [PATCH] fix: accessibility issues on outline and unit pages (#1580)

This PR fixes the following accessibility issues:

1. Header used for screenreader only text
2. Element focus when expanding and dismissing welcome message
3. Bookmark button using wrong ARIA attributing while processing bookmark status
---
 src/course-home/outline-tab/OutlineTab.jsx    | 19 +++---
 .../outline-tab/OutlineTab.test.jsx           | 12 ++++
 .../outline-tab/widgets/WelcomeMessage.jsx    | 61 +++++++++++--------
 .../course/bookmark/BookmarkButton.jsx        | 20 +++---
 .../course/bookmark/BookmarkFilledIcon.jsx    |  7 ---
 .../course/bookmark/BookmarkOutlineIcon.jsx   |  7 ---
 src/courseware/course/bookmark/index.js       |  2 -
 .../Unit/__snapshots__/index.test.jsx.snap    |  4 +-
 src/courseware/course/sequence/Unit/index.jsx |  2 +-
 .../sequence-navigation/UnitButton.jsx        | 10 +--
 .../sequence-navigation/UnitButton.test.jsx   | 10 +--
 11 files changed, 81 insertions(+), 73 deletions(-)
 delete mode 100644 src/courseware/course/bookmark/BookmarkFilledIcon.jsx
 delete mode 100644 src/courseware/course/bookmark/BookmarkOutlineIcon.jsx

diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx
index c0fcfab9..3fddf35a 100644
--- a/src/course-home/outline-tab/OutlineTab.jsx
+++ b/src/course-home/outline-tab/OutlineTab.jsx
@@ -1,9 +1,9 @@
-import React, { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import { useLocation, useNavigate } from 'react-router-dom';
 import { useSelector } from 'react-redux';
 import { sendTrackEvent } from '@edx/frontend-platform/analytics';
 import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
-import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import { useIntl } from '@edx/frontend-platform/i18n';
 import { Button } from '@openedx/paragon';
 import { PluginSlot } from '@openedx/frontend-plugin-framework';
 import { AlertList } from '../../generic/user-messages';
@@ -29,7 +29,8 @@ import WelcomeMessage from './widgets/WelcomeMessage';
 import ProctoringInfoPanel from './widgets/ProctoringInfoPanel';
 import AccountActivationAlert from '../../alerts/logistration-alert/AccountActivationAlert';
 
-const OutlineTab = ({ intl }) => {
+const OutlineTab = () => {
+  const intl = useIntl();
   const {
     courseId,
     proctoringPanelStatus,
@@ -42,6 +43,8 @@ const OutlineTab = ({ intl }) => {
     userTimezone,
   } = useModel('courseHomeMeta', courseId);
 
+  const expandButtonRef = useRef();
+
   const {
     accessExpiration,
     courseBlocks: {
@@ -159,12 +162,12 @@ const OutlineTab = ({ intl }) => {
             </>
           )}
           <StartOrResumeCourseCard />
-          <WelcomeMessage courseId={courseId} />
+          <WelcomeMessage courseId={courseId} nextElementRef={expandButtonRef} />
           {rootCourseId && (
             <>
               <div className="row w-100 m-0 mb-3 justify-content-end">
                 <div className="col-12 col-md-auto p-0">
-                  <Button variant="outline-primary" block onClick={() => { setExpandAll(!expandAll); }}>
+                  <Button ref={expandButtonRef} variant="outline-primary" block onClick={() => { setExpandAll(!expandAll); }}>
                     {expandAll ? intl.formatMessage(messages.collapseAll) : intl.formatMessage(messages.expandAll)}
                   </Button>
                 </div>
@@ -225,8 +228,4 @@ const OutlineTab = ({ intl }) => {
   );
 };
 
-OutlineTab.propTypes = {
-  intl: intlShape.isRequired,
-};
-
-export default injectIntl(OutlineTab);
+export default OutlineTab;
diff --git a/src/course-home/outline-tab/OutlineTab.test.jsx b/src/course-home/outline-tab/OutlineTab.test.jsx
index aace0578..5db8688a 100644
--- a/src/course-home/outline-tab/OutlineTab.test.jsx
+++ b/src/course-home/outline-tab/OutlineTab.test.jsx
@@ -292,6 +292,18 @@ describe('Outline Tab', () => {
         showMoreButton = screen.getByRole('button', { name: 'Show More' });
         expect(showMoreButton).toBeInTheDocument();
       });
+
+      fit('dismisses message', async () => {
+        expect(screen.getByTestId('alert-container-welcome')).toBeInTheDocument();
+        const dismissButton = screen.queryByRole('button', { name: 'Dismiss' });
+        const expandButton = screen.queryByRole('button', { name: 'Expand all' });
+
+        fireEvent.click(dismissButton);
+
+        expect(expandButton).toHaveFocus();
+
+        expect(screen.queryByText('Welcome Message')).toBeNull();
+      });
     });
 
     it('ignores comments and misformatted HTML', async () => {
diff --git a/src/course-home/outline-tab/widgets/WelcomeMessage.jsx b/src/course-home/outline-tab/widgets/WelcomeMessage.jsx
index cca757d6..40bac852 100644
--- a/src/course-home/outline-tab/widgets/WelcomeMessage.jsx
+++ b/src/course-home/outline-tab/widgets/WelcomeMessage.jsx
@@ -1,7 +1,7 @@
-import React, { useState, useMemo } from 'react';
+import { useState, useMemo, useRef } from 'react';
 import PropTypes from 'prop-types';
 
-import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import { useIntl } from '@edx/frontend-platform/i18n';
 import { Alert, Button, TransitionReplace } from '@openedx/paragon';
 import truncate from 'truncate-html';
 
@@ -11,11 +11,13 @@ import messages from '../messages';
 import { useModel } from '../../../generic/model-store';
 import { dismissWelcomeMessage } from '../../data/thunks';
 
-const WelcomeMessage = ({ courseId, intl }) => {
+const WelcomeMessage = ({ courseId, nextElementRef }) => {
+  const intl = useIntl();
   const {
     welcomeMessageHtml,
   } = useModel('outline', courseId);
 
+  const messageBodyRef = useRef();
   const [display, setDisplay] = useState(true);
 
   // welcomeMessageHtml can contain comments or malformatted HTML which can impact the length that determines
@@ -49,13 +51,20 @@ const WelcomeMessage = ({ courseId, intl }) => {
       dismissible
       show={display}
       onClose={() => {
+        nextElementRef.current?.focus();
         setDisplay(false);
         dispatch(dismissWelcomeMessage(courseId));
       }}
       className="raised-card"
       actions={messageCanBeShortened ? [
         <Button
-          onClick={() => setShowShortMessage(!showShortMessage)}
+          onClick={() => {
+            if (showShortMessage) {
+              messageBodyRef.current?.focus();
+            }
+
+            setShowShortMessage(!showShortMessage);
+          }}
           variant="outline-primary"
         >
           {showShortMessage ? intl.formatMessage(messages.welcomeMessageShowMoreButton)
@@ -63,32 +72,34 @@ const WelcomeMessage = ({ courseId, intl }) => {
         </Button>,
       ] : []}
     >
-      <TransitionReplace className="mb-3" enterDuration={400} exitDuration={200}>
-        {showShortMessage ? (
-          <LmsHtmlFragment
-            className="inline-link"
-            data-testid="short-welcome-message-iframe"
-            key="short-html"
-            html={shortWelcomeMessageHtml}
-            title={intl.formatMessage(messages.welcomeMessage)}
-          />
-        ) : (
-          <LmsHtmlFragment
-            className="inline-link"
-            data-testid="long-welcome-message-iframe"
-            key="full-html"
-            html={cleanedWelcomeMessageHtml}
-            title={intl.formatMessage(messages.welcomeMessage)}
-          />
-        )}
-      </TransitionReplace>
+      <div ref={messageBodyRef} tabIndex="-1">
+        <TransitionReplace className="mb-3" enterDuration={400} exitDuration={200}>
+          {showShortMessage ? (
+            <LmsHtmlFragment
+              className="inline-link"
+              data-testid="short-welcome-message-iframe"
+              key="short-html"
+              html={shortWelcomeMessageHtml}
+              title={intl.formatMessage(messages.welcomeMessage)}
+            />
+          ) : (
+            <LmsHtmlFragment
+              className="inline-link"
+              data-testid="long-welcome-message-iframe"
+              key="full-html"
+              html={cleanedWelcomeMessageHtml}
+              title={intl.formatMessage(messages.welcomeMessage)}
+            />
+          )}
+        </TransitionReplace>
+      </div>
     </Alert>
   );
 };
 
 WelcomeMessage.propTypes = {
   courseId: PropTypes.string.isRequired,
-  intl: intlShape.isRequired,
+  nextElementRef: PropTypes.shape({ current: PropTypes.instanceOf(HTMLInputElement) }),
 };
 
-export default injectIntl(WelcomeMessage);
+export default WelcomeMessage;
diff --git a/src/courseware/course/bookmark/BookmarkButton.jsx b/src/courseware/course/bookmark/BookmarkButton.jsx
index 142f5503..54439da1 100644
--- a/src/courseware/course/bookmark/BookmarkButton.jsx
+++ b/src/courseware/course/bookmark/BookmarkButton.jsx
@@ -1,10 +1,9 @@
-import React, { useCallback } from 'react';
+import { useCallback } from 'react';
 import PropTypes from 'prop-types';
-import { StatefulButton } from '@openedx/paragon';
+import { Icon, StatefulButton } from '@openedx/paragon';
 import { FormattedMessage } from '@edx/frontend-platform/i18n';
 import { useDispatch } from 'react-redux';
-import BookmarkOutlineIcon from './BookmarkOutlineIcon';
-import BookmarkFilledIcon from './BookmarkFilledIcon';
+import { Bookmark, BookmarkBorder } from '@openedx/paragon/icons';
 import { removeBookmark, addBookmark } from './data/thunks';
 
 const addBookmarkLabel = (
@@ -42,10 +41,11 @@ const BookmarkButton = ({
   return (
     <StatefulButton
       variant="link"
-      className="px-1 ml-n1 btn-sm text-primary-500"
+      className={`px-1 ml-n1 btn-sm text-primary-500 ${isProcessing && 'disabled'}`}
       onClick={toggleBookmark}
       state={state}
-      disabledStates={['defaultProcessing', 'bookmarkedProcessing']}
+      aria-busy={isProcessing}
+      disabled={isProcessing}
       labels={{
         default: addBookmarkLabel,
         defaultProcessing: addBookmarkLabel,
@@ -53,10 +53,10 @@ const BookmarkButton = ({
         bookmarkedProcessing: hasBookmarkLabel,
       }}
       icons={{
-        default: <BookmarkOutlineIcon className="text-primary" />,
-        defaultProcessing: <BookmarkOutlineIcon className="text-primary" />,
-        bookmarked: <BookmarkFilledIcon className="text-primary" />,
-        bookmarkedProcessing: <BookmarkFilledIcon className="text-primary" />,
+        default: <Icon src={BookmarkBorder} className="text-primary" />,
+        defaultProcessing: <Icon src={BookmarkBorder} className="text-primary" />,
+        bookmarked: <Icon src={Bookmark} className="text-primary" />,
+        bookmarkedProcessing: <Icon src={Bookmark} className="text-primary" />,
       }}
     />
   );
diff --git a/src/courseware/course/bookmark/BookmarkFilledIcon.jsx b/src/courseware/course/bookmark/BookmarkFilledIcon.jsx
deleted file mode 100644
index af6efe73..00000000
--- a/src/courseware/course/bookmark/BookmarkFilledIcon.jsx
+++ /dev/null
@@ -1,7 +0,0 @@
-import React from 'react';
-import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
-import { faBookmark } from '@fortawesome/free-solid-svg-icons';
-
-const BookmarkFilledIcon = (props) => <FontAwesomeIcon icon={faBookmark} {...props} />;
-
-export default BookmarkFilledIcon;
diff --git a/src/courseware/course/bookmark/BookmarkOutlineIcon.jsx b/src/courseware/course/bookmark/BookmarkOutlineIcon.jsx
deleted file mode 100644
index 21536257..00000000
--- a/src/courseware/course/bookmark/BookmarkOutlineIcon.jsx
+++ /dev/null
@@ -1,7 +0,0 @@
-import React from 'react';
-import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
-import { faBookmark } from '@fortawesome/free-regular-svg-icons';
-
-const BookmarkOutlineIcon = (props) => <FontAwesomeIcon icon={faBookmark} {...props} />;
-
-export default BookmarkOutlineIcon;
diff --git a/src/courseware/course/bookmark/index.js b/src/courseware/course/bookmark/index.js
index 23353466..7348a4e7 100644
--- a/src/courseware/course/bookmark/index.js
+++ b/src/courseware/course/bookmark/index.js
@@ -1,3 +1 @@
 export { default as BookmarkButton } from './BookmarkButton';
-export { default as BookmarkFilledIcon } from './BookmarkFilledIcon';
-export { default as BookmarkOutlineIcon } from './BookmarkFilledIcon';
diff --git a/src/courseware/course/sequence/Unit/__snapshots__/index.test.jsx.snap b/src/courseware/course/sequence/Unit/__snapshots__/index.test.jsx.snap
index c82b9fa4..9fa03431 100644
--- a/src/courseware/course/sequence/Unit/__snapshots__/index.test.jsx.snap
+++ b/src/courseware/course/sequence/Unit/__snapshots__/index.test.jsx.snap
@@ -34,11 +34,11 @@ exports[`Unit component output snapshot: not bookmarked, do not show content 1`]
       unitTitle="unit-title"
     />
   </div>
-  <h2
+  <p
     className="sr-only"
   >
     Level 2 headings may be created by course providers in the future.
-  </h2>
+  </p>
   <BookmarkButton
     isBookmarked={false}
     isProcessing={false}
diff --git a/src/courseware/course/sequence/Unit/index.jsx b/src/courseware/course/sequence/Unit/index.jsx
index 4933dedf..04159a89 100644
--- a/src/courseware/course/sequence/Unit/index.jsx
+++ b/src/courseware/course/sequence/Unit/index.jsx
@@ -52,7 +52,7 @@ const Unit = ({
         <h3 className="h3">{unit.title}</h3>
         <UnitTitleSlot courseId={courseId} unitId={id} unitTitle={unit.title} />
       </div>
-      <h2 className="sr-only">{formatMessage(messages.headerPlaceholder)}</h2>
+      <p className="sr-only">{formatMessage(messages.headerPlaceholder)}</p>
       <BookmarkButton
         unitId={unit.id}
         isBookmarked={unit.bookmarked}
diff --git a/src/courseware/course/sequence/sequence-navigation/UnitButton.jsx b/src/courseware/course/sequence/sequence-navigation/UnitButton.jsx
index 3c999189..dbb37582 100644
--- a/src/courseware/course/sequence/sequence-navigation/UnitButton.jsx
+++ b/src/courseware/course/sequence/sequence-navigation/UnitButton.jsx
@@ -1,13 +1,13 @@
-import React, { useCallback } from 'react';
+import { useCallback } from 'react';
 import { Link, useLocation } from 'react-router-dom';
 import PropTypes from 'prop-types';
 import { connect, useSelector } from 'react-redux';
 import classNames from 'classnames';
-import { Button } from '@openedx/paragon';
+import { Button, Icon } from '@openedx/paragon';
+import { Bookmark } from '@openedx/paragon/icons';
 
 import UnitIcon from './UnitIcon';
 import CompleteIcon from './CompleteIcon';
-import BookmarkFilledIcon from '../../bookmark/BookmarkFilledIcon';
 
 const UnitButton = ({
   onClick,
@@ -46,7 +46,9 @@ const UnitButton = ({
       {showTitle && <span className="unit-title">{title}</span>}
       {showCompletion && complete ? <CompleteIcon size="sm" className="text-success ml-2" /> : null}
       {bookmarked ? (
-        <BookmarkFilledIcon
+        <Icon
+          data-testid="bookmark-icon"
+          src={Bookmark}
           className="text-primary small position-absolute"
           style={{ top: '-3px', right: '5px' }}
         />
diff --git a/src/courseware/course/sequence/sequence-navigation/UnitButton.test.jsx b/src/courseware/course/sequence/sequence-navigation/UnitButton.test.jsx
index 2885a215..7a1fdd8b 100644
--- a/src/courseware/course/sequence/sequence-navigation/UnitButton.test.jsx
+++ b/src/courseware/course/sequence/sequence-navigation/UnitButton.test.jsx
@@ -63,17 +63,17 @@ describe('Unit Button', () => {
   });
 
   it('does not show bookmark', () => {
-    const { container } = render(<UnitButton {...mockData} />);
-    container.querySelectorAll('svg').forEach(icon => {
-      expect(icon).not.toHaveClass('fa-bookmark');
-    });
+    const { queryByTestId } = render(<UnitButton {...mockData} />);
+    expect(queryByTestId('bookmark-icon')).toBeNull();
   });
 
   it('shows bookmark', () => {
     const { container } = render(<UnitButton {...mockData} unitId={bookmarkedUnit.id} />, { wrapWithRouter: true });
     const buttonIcons = container.querySelectorAll('svg');
     expect(buttonIcons).toHaveLength(3);
-    expect(buttonIcons[2]).toHaveClass('fa-bookmark');
+
+    const bookmarkIcon = buttonIcons[2].closest('span');
+    expect(bookmarkIcon.getAttribute('data-testid')).toBe('bookmark-icon');
   });
 
   it('handles the click', () => {