From 5288438c9f618abb90d7ff615190ceadc0237cce Mon Sep 17 00:00:00 2001 From: olav Date: Fri, 22 Apr 2022 14:03:58 +0200 Subject: [PATCH] refactor: use buttons for sortable s (#898) * refactor: use buttons for sortable s * refactor: announce sorting to screen readers * refactor: fix MenuItem padding override --- .../AnnouncerContext.test.tsx | 27 +++++++++++ .../AnnouncerContext/AnnouncerContext.tsx | 15 +++++++ .../AnnouncerElement.styles.ts | 13 ++++++ .../AnnouncerElement/AnnouncerElement.tsx | 23 ++++++++++ .../AnnouncerProvider/AnnouncerProvider.tsx | 27 +++++++++++ .../TableCellSortable.styles.ts | 6 +++ .../TableCellSortable/TableCellSortable.tsx | 45 ++++++++++++------- .../FeatureToggleListNew.styles.ts | 6 +++ .../FeatureToggleListNew.tsx | 42 +++++++++++++---- .../NavigationLink/NavigationLink.styles.ts | 5 ++- frontend/src/index.tsx | 11 +++-- 11 files changed, 191 insertions(+), 29 deletions(-) create mode 100644 frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.test.tsx create mode 100644 frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.tsx create mode 100644 frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.styles.ts create mode 100644 frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.tsx create mode 100644 frontend/src/component/common/Announcer/AnnouncerProvider/AnnouncerProvider.tsx diff --git a/frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.test.tsx b/frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.test.tsx new file mode 100644 index 000000000000..b21f1f47c20b --- /dev/null +++ b/frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.test.tsx @@ -0,0 +1,27 @@ +import { render } from 'utils/testRenderer'; +import { AnnouncerProvider } from 'component/common/Announcer/AnnouncerProvider/AnnouncerProvider'; +import { AnnouncerContext } from 'component/common/Announcer/AnnouncerContext/AnnouncerContext'; +import { useContext, useEffect } from 'react'; +import { screen } from '@testing-library/react'; + +test('AnnouncerContext', async () => { + const TestComponent = () => { + const { setAnnouncement } = useContext(AnnouncerContext); + + useEffect(() => { + setAnnouncement('Foo'); + setAnnouncement('Bar'); + }, [setAnnouncement]); + + return null; + }; + + render( + + + + ); + + expect(screen.getByRole('status')).not.toHaveTextContent('Foo'); + expect(screen.getByRole('status')).toHaveTextContent('Bar'); +}); diff --git a/frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.tsx b/frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.tsx new file mode 100644 index 000000000000..5d8e4f429ad4 --- /dev/null +++ b/frontend/src/component/common/Announcer/AnnouncerContext/AnnouncerContext.tsx @@ -0,0 +1,15 @@ +import React from 'react'; + +export interface IAnnouncerContext { + setAnnouncement: React.Dispatch>; +} + +const setAnnouncementPlaceholder = () => { + throw new Error('setAnnouncement called outside AnnouncerContext'); +}; + +// AnnouncerContext announces messages to screen readers through a live region. +// Call setAnnouncement to broadcast a new message to the screen reader. +export const AnnouncerContext = React.createContext({ + setAnnouncement: setAnnouncementPlaceholder, +}); diff --git a/frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.styles.ts b/frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.styles.ts new file mode 100644 index 000000000000..e1faef2878cb --- /dev/null +++ b/frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.styles.ts @@ -0,0 +1,13 @@ +import { makeStyles } from '@material-ui/core/styles'; + +export const useStyles = makeStyles({ + container: { + clip: 'rect(0 0 0 0)', + clipPath: 'inset(50%)', + zIndex: -1, + width: 1, + height: 1, + margin: -1, + padding: 0, + }, +}); diff --git a/frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.tsx b/frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.tsx new file mode 100644 index 000000000000..5bf794934070 --- /dev/null +++ b/frontend/src/component/common/Announcer/AnnouncerElement/AnnouncerElement.tsx @@ -0,0 +1,23 @@ +import React, { ReactElement } from 'react'; +import { useStyles } from 'component/common/Announcer/AnnouncerElement/AnnouncerElement.styles'; + +interface IAnnouncerElementProps { + announcement?: string; +} + +export const AnnouncerElement = ({ + announcement, +}: IAnnouncerElementProps): ReactElement => { + const styles = useStyles(); + + return ( +
+ {announcement} +
+ ); +}; diff --git a/frontend/src/component/common/Announcer/AnnouncerProvider/AnnouncerProvider.tsx b/frontend/src/component/common/Announcer/AnnouncerProvider/AnnouncerProvider.tsx new file mode 100644 index 000000000000..a2fdc63e4f9a --- /dev/null +++ b/frontend/src/component/common/Announcer/AnnouncerProvider/AnnouncerProvider.tsx @@ -0,0 +1,27 @@ +import React, { ReactElement, useMemo, useState, ReactNode } from 'react'; +import { AnnouncerContext } from '../AnnouncerContext/AnnouncerContext'; +import { AnnouncerElement } from 'component/common/Announcer/AnnouncerElement/AnnouncerElement'; + +interface IAnnouncerProviderProps { + children: ReactNode; +} + +export const AnnouncerProvider = ({ + children, +}: IAnnouncerProviderProps): ReactElement => { + const [announcement, setAnnouncement] = useState(); + + const value = useMemo( + () => ({ + setAnnouncement, + }), + [setAnnouncement] + ); + + return ( + + {children} + + + ); +}; diff --git a/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.styles.ts b/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.styles.ts index 112ee30bbc35..6eefb5294320 100644 --- a/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.styles.ts +++ b/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.styles.ts @@ -22,4 +22,10 @@ export const useStyles = makeStyles(theme => ({ }, }, }, + sortButton: { + all: 'unset', + '&:focus-visible, &:active': { + outline: 'revert', + }, + }, })); diff --git a/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.tsx b/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.tsx index 7870e9f94ee9..2d8f7054ce24 100644 --- a/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.tsx +++ b/frontend/src/component/common/Table/TableCellSortable/TableCellSortable.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode } from 'react'; +import React, { ReactNode, useContext } from 'react'; import { TableCell } from '@material-ui/core'; import classnames from 'classnames'; import { @@ -9,6 +9,7 @@ import { import { IUsersSort, UsersSortType } from 'hooks/useUsersSort'; import ConditionallyRender from 'component/common/ConditionallyRender'; import { useStyles } from 'component/common/Table/TableCellSortable/TableCellSortable.styles'; +import { AnnouncerContext } from 'component/common/Announcer/AnnouncerContext/AnnouncerContext'; // Add others as needed, e.g. UsersSortType | FeaturesSortType type SortType = UsersSortType; @@ -29,23 +30,37 @@ export const TableCellSortable = ({ setSort, children, }: ITableCellSortableProps) => { + const { setAnnouncement } = useContext(AnnouncerContext); const styles = useStyles(); + const ariaSort = + sort.type === name + ? sort.desc + ? 'descending' + : 'ascending' + : undefined; + + const cellClassName = classnames( + className, + styles.tableCellHeaderSortable, + sort.type === name && 'sorted' + ); + + const onSortClick = () => { + setSort(prev => ({ + desc: !Boolean(prev.desc), + type: name, + })); + setAnnouncement( + `Sorted table by ${name}, ${sort.desc ? 'ascending' : 'descending'}` + ); + }; + return ( - - setSort(prev => ({ - desc: !Boolean(prev.desc), - type: name, - })) - } - > - {children} + + ({ color: theme.palette.primary.main, fontWeight: theme.fontWeight.bold, }, + sortButton: { + all: 'unset', + '&:focus-visible, &:active': { + outline: 'revert', + }, + }, })); diff --git a/frontend/src/component/feature/FeatureToggleListNew/FeatureToggleListNew.tsx b/frontend/src/component/feature/FeatureToggleListNew/FeatureToggleListNew.tsx index 81664e0faf58..741e1999e214 100644 --- a/frontend/src/component/feature/FeatureToggleListNew/FeatureToggleListNew.tsx +++ b/frontend/src/component/feature/FeatureToggleListNew/FeatureToggleListNew.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useContext } from 'react'; import { Table, TableBody, @@ -18,6 +18,7 @@ import { import PaginateUI from 'component/common/PaginateUI/PaginateUI'; import StringTruncator from 'component/common/StringTruncator/StringTruncator'; import { createGlobalStateHook } from 'hooks/useGlobalState'; +import { AnnouncerContext } from 'component/common/Announcer/AnnouncerContext/AnnouncerContext'; interface IFeatureToggleListNewProps { features: IFeatureToggleListItem[]; loading: boolean; @@ -83,6 +84,7 @@ const FeatureToggleListNew = ({ projectId, }: IFeatureToggleListNewProps) => { const styles = useStyles(); + const { setAnnouncement } = useContext(AnnouncerContext); const [sortOpt, setSortOpt] = useFeatureToggLeProjectSort(); const [sortedFeatures, setSortedFeatures] = useState( sortList([...features], sortOpt) @@ -116,6 +118,12 @@ const FeatureToggleListNew = ({ setSortOpt(newSortOpt); setSortedFeatures(sortList([...features], newSortOpt)); setPageIndex(0); + + setAnnouncement( + `Sorted table by ${field}, ${ + sortOpt.direction ? 'ascending' : 'descending' + }` + ); }; const getEnvironments = () => { @@ -163,6 +171,14 @@ const FeatureToggleListNew = ({ }); }; + const ariaSort = (field: string) => { + return field === sortOpt.field + ? sortOpt.direction + ? 'ascending' + : 'descending' + : undefined; + }; + return ( <> @@ -176,13 +192,15 @@ const FeatureToggleListNew = ({ styles.tableCellHeaderSortable )} align="left" + aria-sort={ariaSort('lastSeenAt')} > - updateSort('lastSeenAt')} + className={styles.sortButton} > Last use - + - updateSort('type')} + className={styles.sortButton} > Type - + - updateSort('name')} + className={styles.sortButton} > Name - + - updateSort('createdAt')} + className={styles.sortButton} > Created - + {getEnvironments().map((env: any) => { return ( diff --git a/frontend/src/component/menu/Header/NavigationLink/NavigationLink.styles.ts b/frontend/src/component/menu/Header/NavigationLink/NavigationLink.styles.ts index 06021ecf6b19..180d5651becb 100644 --- a/frontend/src/component/menu/Header/NavigationLink/NavigationLink.styles.ts +++ b/frontend/src/component/menu/Header/NavigationLink/NavigationLink.styles.ts @@ -23,6 +23,9 @@ export const useStyles = makeStyles(theme => ({ color: '#000', height: '100%', width: '100%', - padding: '0.5rem 1rem', + '&&': { + // Override MenuItem's built-in padding. + padding: '0.5rem 1rem', + }, }, })); diff --git a/frontend/src/index.tsx b/frontend/src/index.tsx index f6d992092b2f..932e1a7187be 100644 --- a/frontend/src/index.tsx +++ b/frontend/src/index.tsx @@ -12,6 +12,7 @@ import AccessProvider from 'component/providers/AccessProvider/AccessProvider'; import { getBasePath } from 'utils/formatPath'; import { FeedbackCESProvider } from 'component/feedback/FeedbackCESContext/FeedbackCESProvider'; import UIProvider from 'component/providers/UIProvider/UIProvider'; +import { AnnouncerProvider } from 'component/common/Announcer/AnnouncerProvider/AnnouncerProvider'; ReactDOM.render( @@ -19,10 +20,12 @@ ReactDOM.render( - - - - + + + + + +