From 4b38aaa199737fee66a5bfcb068aa6888d300f6c Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Wed, 19 Oct 2022 15:01:50 -0400 Subject: [PATCH] fix: a11y issues (#47) --- package-lock.json | 40 +++++ package.json | 1 + src/App.jsx | 7 + src/__snapshots__/App.test.jsx.snap | 8 + .../components/CourseCardContent.jsx | 8 +- .../__snapshots__/index.test.jsx.snap | 2 +- .../components/CourseCardMenu/index.jsx | 2 +- .../CourseCardContent.test.jsx.snap | 28 ++-- .../__snapshots__/index.test.jsx.snap | 12 -- src/containers/CourseList/hooks.js | 3 - src/containers/CourseList/hooks.test.js | 6 - src/containers/CourseList/index.jsx | 9 +- src/containers/CourseList/index.test.jsx | 5 - .../__snapshots__/index.test.jsx.snap | 64 ++++++-- src/containers/Dashboard/index.jsx | 26 +++- src/containers/Dashboard/index.test.jsx | 13 +- .../LearnerDashboardHeader/GreetingBanner.jsx | 18 ++- .../GreetingBanner.test.jsx.snap | 138 +++++++++++------- .../__snapshots__/index.test.jsx.snap | 2 + .../components/ProgramCard.jsx | 3 +- .../__snapshots__/ProgramCard.test.jsx.snap | 5 +- .../components/messages.js | 4 +- src/containers/RelatedProgramsModal/index.jsx | 2 +- src/messages.js | 16 ++ 24 files changed, 284 insertions(+), 138 deletions(-) create mode 100644 src/messages.js diff --git a/package-lock.json b/package-lock.json index c2d8d8d..56a750f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40,6 +40,7 @@ "query-string": "7.0.1", "react": "^16.14.0", "react-dom": "^16.14.0", + "react-helmet": "^6.1.0", "react-intl": "^5.20.9", "react-pdf": "^5.5.0", "react-redux": "^7.2.4", @@ -24531,6 +24532,20 @@ } } }, + "node_modules/react-helmet": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/react-helmet/-/react-helmet-6.1.0.tgz", + "integrity": "sha512-4uMzEY9nlDlgxr61NL3XbKRy1hEkXmKNXhjbAIOVw5vcFrsdYbH2FEwcNyWvWinl103nXgzYNlns9ca+8kFiWw==", + "dependencies": { + "object-assign": "^4.1.1", + "prop-types": "^15.7.2", + "react-fast-compare": "^3.1.1", + "react-side-effect": "^2.1.0" + }, + "peerDependencies": { + "react": ">=16.3.0" + } + }, "node_modules/react-intl": { "version": "5.25.1", "resolved": "https://registry.npmjs.org/react-intl/-/react-intl-5.25.1.tgz", @@ -24870,6 +24885,14 @@ "isarray": "0.0.1" } }, + "node_modules/react-side-effect": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/react-side-effect/-/react-side-effect-2.1.2.tgz", + "integrity": "sha512-PVjOcvVOyIILrYoyGEpDN3vmYNLdy1CajSFNt4TDsVQC5KpTijDvWVoR+/7Rz2xT978D8/ZtFceXxzsPwZEDvw==", + "peerDependencies": { + "react": "^16.3.0 || ^17.0.0 || ^18.0.0" + } + }, "node_modules/react-style-singleton": { "version": "2.2.1", "resolved": "https://registry.npmjs.org/react-style-singleton/-/react-style-singleton-2.2.1.tgz", @@ -48625,6 +48648,17 @@ "use-sidecar": "^1.1.2" } }, + "react-helmet": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/react-helmet/-/react-helmet-6.1.0.tgz", + "integrity": "sha512-4uMzEY9nlDlgxr61NL3XbKRy1hEkXmKNXhjbAIOVw5vcFrsdYbH2FEwcNyWvWinl103nXgzYNlns9ca+8kFiWw==", + "requires": { + "object-assign": "^4.1.1", + "prop-types": "^15.7.2", + "react-fast-compare": "^3.1.1", + "react-side-effect": "^2.1.0" + } + }, "react-intl": { "version": "5.25.1", "resolved": "https://registry.npmjs.org/react-intl/-/react-intl-5.25.1.tgz", @@ -48888,6 +48922,12 @@ } } }, + "react-side-effect": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/react-side-effect/-/react-side-effect-2.1.2.tgz", + "integrity": "sha512-PVjOcvVOyIILrYoyGEpDN3vmYNLdy1CajSFNt4TDsVQC5KpTijDvWVoR+/7Rz2xT978D8/ZtFceXxzsPwZEDvw==", + "requires": {} + }, "react-style-singleton": { "version": "2.2.1", "resolved": "https://registry.npmjs.org/react-style-singleton/-/react-style-singleton-2.2.1.tgz", diff --git a/package.json b/package.json index 1b565ce..de05468 100755 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "query-string": "7.0.1", "react": "^16.14.0", "react-dom": "^16.14.0", + "react-helmet": "^6.1.0", "react-intl": "^5.20.9", "react-pdf": "^5.5.0", "react-redux": "^7.2.4", diff --git a/src/App.jsx b/src/App.jsx index 672ecae..2ea4fa5 100755 --- a/src/App.jsx +++ b/src/App.jsx @@ -1,7 +1,9 @@ import React from 'react'; import { BrowserRouter as Router } from 'react-router-dom'; import { useDispatch } from 'react-redux'; +import { Helmet } from 'react-helmet'; +import { useIntl } from '@edx/frontend-platform/i18n'; import { AppContext } from '@edx/frontend-platform/react'; import Footer from '@edx/frontend-component-footer'; @@ -9,6 +11,7 @@ import { thunkActions } from 'data/redux'; import fakeData from 'data/services/lms/fakeData/courses'; import LearnerDashboardHeader from 'containers/LearnerDashboardHeader'; import Dashboard from 'containers/Dashboard'; +import messages from './messages'; import './App.scss'; @@ -16,6 +19,7 @@ export const App = () => { const dispatch = useDispatch(); // TODO: made development-only const { authenticatedUser } = React.useContext(AppContext); + const { formatMessage } = useIntl(); React.useEffect(() => { if (authenticatedUser?.administrator || process.env.NODE_ENV === 'development') { window.loadEmptyData = () => { @@ -34,6 +38,9 @@ export const App = () => { }); return ( + + {formatMessage(messages.pageTitle)} +
diff --git a/src/__snapshots__/App.test.jsx.snap b/src/__snapshots__/App.test.jsx.snap index 0748d92..59c2da2 100644 --- a/src/__snapshots__/App.test.jsx.snap +++ b/src/__snapshots__/App.test.jsx.snap @@ -2,6 +2,14 @@ exports[`App router component snapshot: enabled 1`] = ` + + + Learner Home + +
diff --git a/src/containers/CourseCard/components/CourseCardContent.jsx b/src/containers/CourseCard/components/CourseCardContent.jsx index df024bd..de3a11e 100644 --- a/src/containers/CourseCard/components/CourseCardContent.jsx +++ b/src/containers/CourseCard/components/CourseCardContent.jsx @@ -30,9 +30,11 @@ export const CourseCardContent = ({ cardId, orientation }) => { - {courseName} - +

+ + {courseName} + +

)} actions={} /> diff --git a/src/containers/CourseCard/components/CourseCardMenu/__snapshots__/index.test.jsx.snap b/src/containers/CourseCard/components/CourseCardMenu/__snapshots__/index.test.jsx.snap index ec4510b..6b4593a 100644 --- a/src/containers/CourseCard/components/CourseCardMenu/__snapshots__/index.test.jsx.snap +++ b/src/containers/CourseCard/components/CourseCardMenu/__snapshots__/index.test.jsx.snap @@ -7,7 +7,7 @@ exports[`CourseCardMenu snapshot 1`] = ` alt="Actions dropdown" as="IconButton" iconAs="Icon" - id="dropdown-toggle-with-iconbutton" + id="course-actions-dropdown-test-card-id" src={[MockFunction icons.MoreVert]} variant="primary" /> diff --git a/src/containers/CourseCard/components/CourseCardMenu/index.jsx b/src/containers/CourseCard/components/CourseCardMenu/index.jsx index f24b14d..4d9f03a 100644 --- a/src/containers/CourseCard/components/CourseCardMenu/index.jsx +++ b/src/containers/CourseCard/components/CourseCardMenu/index.jsx @@ -19,7 +19,7 @@ export const CourseCardMenu = ({ cardId }) => { <> } title={ - - test-course-name - +

+ + test-course-name + +

} /> } title={ - - test-course-name - +

+ + test-course-name + +

} /> - -
-`; - exports[`CourseList snapshots with filters 1`] = `
{ }); const handleRemoveFilter = (filter) => () => setFilters.remove(filter); const setPageNumber = (value) => dispatch(actions.app.setPageNumber(value)); - const initIsPending = appHooks.useIsPendingRequest(RequestKeys.initialize); return { pageNumber, @@ -42,7 +40,6 @@ export const useCourseListData = () => { handleRemoveFilter, }, showFilters: filters.length > 0, - initIsPending, }; }; diff --git a/src/containers/CourseList/hooks.test.js b/src/containers/CourseList/hooks.test.js index 7a90ab5..c22c13b 100644 --- a/src/containers/CourseList/hooks.test.js +++ b/src/containers/CourseList/hooks.test.js @@ -80,12 +80,6 @@ describe('CourseList hooks', () => { // don't show filter when list is empty. expect(out.showFilters).toEqual(false); }); - test('initIsPending loads from useIsPendingRequest', () => { - expect(out.initIsPending).toEqual(false); - appHooks.useIsPendingRequest.mockReturnValueOnce(true); - out = hooks.useCourseListData(); - expect(out.initIsPending).toEqual(true); - }); describe('filterOptions', () => { test('sortBy and setSortBy are connected to the state value', () => { expect(out.filterOptions.sortBy).toEqual(testSortBy); diff --git a/src/containers/CourseList/index.jsx b/src/containers/CourseList/index.jsx index 88ff7a1..be37b77 100644 --- a/src/containers/CourseList/index.jsx +++ b/src/containers/CourseList/index.jsx @@ -1,7 +1,7 @@ import React from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { Pagination, Spinner } from '@edx/paragon'; +import { Pagination } from '@edx/paragon'; import { ActiveCourseFilters, @@ -23,13 +23,8 @@ export const CourseList = () => { numPages, showFilters, visibleList, - initIsPending, } = useCourseListData(); - return initIsPending ? ( -
- -
- ) : ( + return (

{formatMessage(messages.myCourses)}

diff --git a/src/containers/CourseList/index.test.jsx b/src/containers/CourseList/index.test.jsx index b848583..691a552 100644 --- a/src/containers/CourseList/index.test.jsx +++ b/src/containers/CourseList/index.test.jsx @@ -20,7 +20,6 @@ describe('CourseList', () => { setPageNumber: jest.fn().mockName('setPageNumber'), showFilters: false, visibleList: [], - initIsPending: false, }; const createWrapper = (courseListData) => { useCourseListData.mockReturnValueOnce({ @@ -31,10 +30,6 @@ describe('CourseList', () => { }; describe('snapshots', () => { - it('renders loading', () => { - const wrapper = createWrapper({ initIsPending: true }); - expect(wrapper).toMatchSnapshot(); - }); test('with no filters', () => { const wrapper = createWrapper(); expect(wrapper).toMatchSnapshot(); diff --git a/src/containers/Dashboard/__snapshots__/index.test.jsx.snap b/src/containers/Dashboard/__snapshots__/index.test.jsx.snap index 30cc0f3..1af60b4 100644 --- a/src/containers/Dashboard/__snapshots__/index.test.jsx.snap +++ b/src/containers/Dashboard/__snapshots__/index.test.jsx.snap @@ -1,20 +1,15 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Dashboard snapshots there are available dashboards 1`] = ` -
- - -
-`; - -exports[`Dashboard snapshots there are courses, or they are still loading 1`] = ` +exports[`Dashboard snapshots courses loaded 1`] = `
+

+ Learner Home +

`; +exports[`Dashboard snapshots courses still loading 1`] = ` +
+

+ Learner Home +

+
+ +
+
+`; + +exports[`Dashboard snapshots there are available dashboards 1`] = ` +
+

+ Learner Home +

+ + +
+`; + exports[`Dashboard snapshots there are no courses 1`] = `
+

+ Learner Home +

`; @@ -81,6 +118,11 @@ exports[`Dashboard snapshots there is a select session modal 1`] = ` className="d-flex flex-column p-2" id="dashboard-container" > +

+ Learner Home +

`; diff --git a/src/containers/Dashboard/index.jsx b/src/containers/Dashboard/index.jsx index 3d7ef02..5b656e6 100644 --- a/src/containers/Dashboard/index.jsx +++ b/src/containers/Dashboard/index.jsx @@ -1,6 +1,12 @@ import React from 'react'; import { useDispatch } from 'react-redux'; -import { Container, Col, Row } from '@edx/paragon'; +import { + Container, + Col, + Row, + Spinner, +} from '@edx/paragon'; +import { useIntl } from '@edx/frontend-platform/i18n'; import { thunkActions, @@ -14,6 +20,8 @@ import EmptyCourse from 'containers/EmptyCourse'; import SelectSessionModal from 'containers/SelectSessionModal'; import EnterpriseDashboardModal from 'containers/EnterpriseDashboardModal'; +import appMessages from 'messages'; + import './index.scss'; export const Dashboard = () => { @@ -22,6 +30,7 @@ export const Dashboard = () => { () => { dispatch(thunkActions.app.initialize()); }, [dispatch], ); + const { formatMessage } = useIntl(); const hasCourses = appHooks.useHasCourses(); const hasAvailableDashboards = appHooks.useHasAvailableDashboards(); @@ -30,8 +39,18 @@ export const Dashboard = () => { return (
+

{formatMessage(appMessages.pageTitle)}

{hasAvailableDashboards && } - {initIsPending || (!initIsPending && hasCourses) ? ( + {initIsPending && ( +
+ +
+ )} + {(!initIsPending && hasCourses) && ( { - ) : ()} + )} + {(!initIsPending && !hasCourses) && ()}
); }; diff --git a/src/containers/Dashboard/index.test.jsx b/src/containers/Dashboard/index.test.jsx index f18d644..02f1eab 100644 --- a/src/containers/Dashboard/index.test.jsx +++ b/src/containers/Dashboard/index.test.jsx @@ -45,22 +45,23 @@ describe('Dashboard', () => { }; describe('snapshots', () => { - test('there are courses, or they are still loading', () => { - const pendingNoCoursesWrapper = createWrapper({ + test('courses still loading', () => { + const wrapper = createWrapper({ hasCourses: false, hasAvailableDashboards: false, showSelectSessionModal: false, initIsPending: true, }); - expect(pendingNoCoursesWrapper).toMatchSnapshot(); - - const doneLoadingWithCoursesWrapper = createWrapper({ + expect(wrapper).toMatchSnapshot(); + }); + test('courses loaded', () => { + const wrapper = createWrapper({ hasCourses: true, hasAvailableDashboards: false, showSelectSessionModal: false, initIsPending: false, }); - expect(doneLoadingWithCoursesWrapper).toEqual(pendingNoCoursesWrapper); + expect(wrapper).toMatchSnapshot(); }); test('there are no courses', () => { diff --git a/src/containers/LearnerDashboardHeader/GreetingBanner.jsx b/src/containers/LearnerDashboardHeader/GreetingBanner.jsx index cddf1dd..a4ec306 100644 --- a/src/containers/LearnerDashboardHeader/GreetingBanner.jsx +++ b/src/containers/LearnerDashboardHeader/GreetingBanner.jsx @@ -31,20 +31,22 @@ export const GreetingBanner = ({ size }) => { { 'p-5': !isSmall, 'p-3.5': isSmall }, )} > - {getConfig().SITE_NAME} + + {getConfig().SITE_NAME} +
{isSmall ? ( -
+
{formatMessage(greetMessage)}
) : ( -

+

{formatMessage(greetMessage)}

)} diff --git a/src/containers/LearnerDashboardHeader/__snapshots__/GreetingBanner.test.jsx.snap b/src/containers/LearnerDashboardHeader/__snapshots__/GreetingBanner.test.jsx.snap index 3b341cf..004e809 100644 --- a/src/containers/LearnerDashboardHeader/__snapshots__/GreetingBanner.test.jsx.snap +++ b/src/containers/LearnerDashboardHeader/__snapshots__/GreetingBanner.test.jsx.snap @@ -4,21 +4,26 @@ exports[`GreetingBanner snapshots with size large and afternoon 1`] = `
- localhost + localhost + /> +

Good Afternoon!

@@ -29,21 +34,26 @@ exports[`GreetingBanner snapshots with size large and evening 1`] = `
- localhost + localhost + /> +

Good Evening!

@@ -54,21 +64,26 @@ exports[`GreetingBanner snapshots with size large and morning 1`] = `
- localhost + localhost + /> +

Good Morning!

@@ -79,21 +94,26 @@ exports[`GreetingBanner snapshots with size small and afternoon 1`] = `
- localhost + localhost + /> +
Good Afternoon!
@@ -104,21 +124,26 @@ exports[`GreetingBanner snapshots with size small and evening 1`] = `
- localhost + localhost + /> +
Good Evening!
@@ -129,21 +154,26 @@ exports[`GreetingBanner snapshots with size small and morning 1`] = `
- localhost + localhost + /> +
Good Morning!
diff --git a/src/containers/RelatedProgramsModal/__snapshots__/index.test.jsx.snap b/src/containers/RelatedProgramsModal/__snapshots__/index.test.jsx.snap index 5298e8a..980e5fb 100644 --- a/src/containers/RelatedProgramsModal/__snapshots__/index.test.jsx.snap +++ b/src/containers/RelatedProgramsModal/__snapshots__/index.test.jsx.snap @@ -12,6 +12,7 @@ exports[`RelatedProgramsModal snapshot: closed 1`] = ` title="Related Programs" > @@ -95,6 +96,7 @@ exports[`RelatedProgramsModal snapshot: open 1`] = ` title="Related Programs" > diff --git a/src/containers/RelatedProgramsModal/components/ProgramCard.jsx b/src/containers/RelatedProgramsModal/components/ProgramCard.jsx index b05aaac..a0e8c3f 100644 --- a/src/containers/RelatedProgramsModal/components/ProgramCard.jsx +++ b/src/containers/RelatedProgramsModal/components/ProgramCard.jsx @@ -27,13 +27,14 @@ export const ProgramCard = ({ data }) => { style={{ width: '18rem', color: 'white' }} as="a" href={data.programUrl} + isClickable > - + {formatMessage(messages.header)} diff --git a/src/messages.js b/src/messages.js new file mode 100644 index 0000000..a1c7a19 --- /dev/null +++ b/src/messages.js @@ -0,0 +1,16 @@ +import { StrictDict } from 'utils'; + +export const messages = StrictDict({ + loadingSR: { + id: 'learner-dash.loadingSR', + description: 'Page loading screen-reader text', + defaultMessage: 'Loading...', + }, + pageTitle: { + id: 'learner-dash.title', + description: 'Page title: Learner Home', + defaultMessage: 'Learner Home', + }, +}); + +export default messages;