Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

Enhance screen reader support to announce column headers are interactable. #1836

Merged
merged 74 commits into from
Oct 25, 2023
Merged
Changes from 71 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
4586b1b
Copy src from branch UXPLATFORM-9147
sycombs Sep 25, 2023
61e616e
Copy docs/tests from UXPLATFORM-9147 branch
sycombs Sep 25, 2023
41d5697
Copy changelog entry
sycombs Sep 25, 2023
f0c547b
Copy the Jest updates
sycombs Sep 25, 2023
d0e71e5
Add translations
sycombs Sep 25, 2023
f83a0d8
Merge remote-tracking branch 'origin/main' into wdg_column_resizing
sycombs Sep 25, 2023
d2c2959
role 'slider' changed to be null when slider is not active
adoroshk Sep 29, 2023
0cc9d6e
Merge branch 'main' into wdg_column_resizing
adoroshk Sep 29, 2023
6356750
aria-label and aria-valuetext adjusted, role reverted
adoroshk Sep 29, 2023
c64ad23
slider - divider interchangable roles
adoroshk Oct 2, 2023
0dad843
use isNavigationEnabled for role
adoroshk Oct 2, 2023
5568179
testing without aria-valuetext, aria-label only
adoroshk Oct 2, 2023
b727ebe
combined aria-label
adoroshk Oct 2, 2023
2a4ceeb
aria-label and aria-valuetext adjusted
adoroshk Oct 2, 2023
ff48e99
added cell-focus-trapped and resume-navigation texts
adoroshk Oct 2, 2023
1b15e28
enabling navigation after setting column header aria live message
adoroshk Oct 2, 2023
78b23ed
removed columnWidthAnnouncement from aria-label
adoroshk Oct 3, 2023
a4fd285
Merge branch 'main' into wdg_column_resizing
adoroshk Oct 3, 2023
835c69d
state variables for aria-label and aria-valuetext added
adoroshk Oct 3, 2023
bc74a4a
ariaLabel and ariaValueText switched to boolean
adoroshk Oct 3, 2023
e2d3d3b
aria-label assigned on focus and removed on Enter or onBlur
adoroshk Oct 3, 2023
4de9bae
revert table prop changes
sdadn Oct 5, 2023
c949b32
updated ColumnResizeHandle.scss
sdadn Oct 5, 2023
82615a0
removed tableheight
sdadn Oct 6, 2023
526698c
removed table height calculation
sdadn Oct 6, 2023
04a321e
reverted condition
sdadn Oct 6, 2023
010c3f9
revert colors
sdadn Oct 6, 2023
ca10df4
Removed space
sdadn Oct 6, 2023
9569340
linter fixes
sdadn Oct 6, 2023
52c788e
linter fixes
sdadn Oct 6, 2023
ce7f8ea
Merge branch 'main' into wdg_column_resizing
sdadn Oct 6, 2023
fb573e5
added column resize handle tests
sdadn Oct 9, 2023
bc4901e
column header cell updates
sdadn Oct 9, 2023
a3320ed
linter fixes
sdadn Oct 9, 2023
db90402
updated column header cell snapshots
sdadn Oct 11, 2023
d5c6ea0
updated snapshots
sdadn Oct 11, 2023
256a9f5
fix for role slider or no role at all
adoroshk Oct 11, 2023
e1b6ddf
Merge branch 'main' into wdg_column_resizing
adoroshk Oct 11, 2023
b28d215
added wdio tests
sdadn Oct 13, 2023
7f5cf5d
merged main into current branch
sdadn Oct 13, 2023
c4b58f1
fixed jest tests
sdadn Oct 13, 2023
955a02e
linter fixes
sdadn Oct 13, 2023
698d130
updated datagrid test
sdadn Oct 13, 2023
b634939
Merge branch 'main' into wdg_column_resizing
sdadn Oct 13, 2023
48a60cc
Apply suggestions from code review
sdadn Oct 16, 2023
e4be9b6
Update packages/terra-data-grid/src/subcomponents/ColumnResizeHandle.jsx
sdadn Oct 16, 2023
84f17e0
Changed to focus button in the column header
Oct 16, 2023
30e3985
Update for github actions. Will be reverted later
Oct 16, 2023
e5fd211
Merge branch 'main' into wdg_column_resizing
sdadn Oct 17, 2023
6e66593
updated datagrid
sdadn Oct 17, 2023
281cf72
Updated selector for tests
Oct 18, 2023
74fb315
fixed isResizeHandleActive not resetting
sdadn Oct 18, 2023
5868be5
working column resizing
sdadn Oct 18, 2023
cb29ed1
updated tests
sdadn Oct 19, 2023
58ba111
working tests
sdadn Oct 20, 2023
aac35bf
linter fixes
sdadn Oct 20, 2023
9340a4c
removed polyfill
sdadn Oct 20, 2023
b81bf75
updated snapshots
sdadn Oct 20, 2023
0fc61be
updated snapshots
sdadn Oct 20, 2023
7cd98b5
updated snapshots
sdadn Oct 20, 2023
f74b948
updated CHANGELOG
sdadn Oct 20, 2023
00353ed
linter fixes
sdadn Oct 20, 2023
670288f
Merge branch 'main' into wdg_column_resizing
sdadn Oct 20, 2023
e149d55
updates wdio snapshots
sdadn Oct 20, 2023
88581fc
Merge branch 'wdg_column_resizing' into UXPLATFORM-9611
Oct 23, 2023
a2e12e1
Changes to focus button for column header cell
Oct 23, 2023
96e9da9
Merge branch 'wdg_column_resizing' into UXPLATFORM-9611
Oct 23, 2023
7de1a76
Added tests and fixed console, build errors
Oct 23, 2023
0a74744
Merge branch 'wdg_column_resizing' into UXPLATFORM-9611
Oct 24, 2023
4fa2502
Updated WDIO screenshots for new resize handle styles
Oct 24, 2023
016ab65
Added Changelog for documentation update.
Oct 24, 2023
ad6ad83
Merge branch 'wdg_column_resizing' into UXPLATFORM-9611
brithom Oct 24, 2023
15bc0e1
fix snapshop
brithom Oct 24, 2023
fe7af39
Merge branch 'wdg_column_resizing' into UXPLATFORM-9611
brithom Oct 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
@@ -3,9 +3,9 @@ name: CI and CD

on:
push:
branches: ['main']
branches: ['wdg_column_resizing']
pull_request:
branches: ['main']
branches: ['wdg_column_resizing']
schedule:
- cron: '0 1 * * SUN'
workflow_dispatch:
2 changes: 1 addition & 1 deletion .github/workflows/pr-preview.yml
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ name: pr-preview

on:
pull_request:
branches: ['main']
branches: ['wdg_column_resizing']
jobs:
deploy:
runs-on: ubuntu-latest
4 changes: 4 additions & 0 deletions packages/terra-data-grid/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Changelog

## Unreleased
* Added
* Added additional screen reader support to announce that column headers are interactable upon selection.


* Added
* Added keyboard support for column resizing.
@@ -17,6 +20,7 @@
## 0.8.0 - (October 11, 2023)

* Added
* Added ability to resize columns via keyboard.
* Added base FlowsheetDataGrid component.

* Changed
11 changes: 10 additions & 1 deletion packages/terra-data-grid/src/DataGrid.jsx
Original file line number Diff line number Diff line change
@@ -189,6 +189,7 @@ const DataGrid = injectIntl((props) => {
const [checkResizable, setCheckResizable] = useState(false);
const [focusedRow, setFocusedRow] = useState(0);
const [focusedCol, setFocusedCol] = useState(0);
const [gridHasFocus, setGridHasFocus] = useState(false);

// Aria live region message management
const [columnHeaderAriaLiveMessage, setColumnHeaderAriaLiveMessage] = useState(null);
@@ -584,12 +585,19 @@ const DataGrid = injectIntl((props) => {
// Not triggered when swapping focus between children
if (handleFocus.current) {
setFocusedRowCol(focusedRow, focusedCol, true);
setGridHasFocus(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think that we could determine this information without needing to alter the focus event handler logic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where this logic should live. When the grid is initially rendered, nothing on the grid has focus. All the columnHeaderCells would be rendered with isActive=false. When the grid gets focus, since nothing is getting re-rendered, the state of the columnHeaderCell that got focus is incorrect. This ensures that when the grid gets focus the columnHeaderCell with the focus gets updated correctly.

}
}

handleFocus.current = true;
};

const onBlur = (event) => {
if (!event.currentTarget.contains(event.relatedTarget)) {
setGridHasFocus(false);
}
};
Comment on lines +595 to +599
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not need to add new event handler logic to determine if the grid has focus.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the grid loses focus, we want the columnHeaderCell that had focus to reflect the correct state.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the column header cell would lose focus itself. We should not need a state update because the CSS is based on focus, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The columnHeaderCell's active state is set via a prop so if we do not do this here, the columnHeaderCell would have lost focus but its active state would remain true. The result is that when focus comes back into the grid, the columnHeaderCell will not get re-rendered thus causing incorrect screen reader behavior because focus is not correctly set to the button.


// -------------------------------------

return (
@@ -603,6 +611,7 @@ const DataGrid = injectIntl((props) => {
className={cx('data-grid', theme.className)}
onKeyDown={handleKeyDown}
onFocus={onFocus}
onBlur={onBlur}
onMouseDown={onMouseDown}
tabIndex={0}
{...(activeIndex != null && { onMouseUp, onMouseMove, onMouseLeave: onMouseUp })}
@@ -614,7 +623,7 @@ const DataGrid = injectIntl((props) => {
columns={dataGridColumns}
headerHeight={columnHeaderHeight}
tableHeight={tableHeight}
activeColumnIndex={focusedRow === 0 ? focusedCol : undefined}
activeColumnIndex={(gridHasFocus && focusedRow === 0) ? focusedCol : undefined}
activeColumnResizing={focusedRow === 0 && checkResizable}
columnResizeIncrement={columnResizeIncrement}
onColumnSelect={handleColumnSelect}
2 changes: 1 addition & 1 deletion packages/terra-data-grid/src/WorklistDataGrid.jsx
Original file line number Diff line number Diff line change
@@ -412,7 +412,7 @@ function WorklistDataGrid(props) {
onKeyDown={handleKeyDown}
onKeyUp={handleKeyUp}
className={cx('worklist-data-grid-container')}
onFocus={!gridReceivedFocus.current && onFocus}
onFocus={!gridReceivedFocus.current ? onFocus : undefined}
>
<DataGrid
id={id}
25 changes: 18 additions & 7 deletions packages/terra-data-grid/src/subcomponents/ColumnHeaderCell.jsx
Original file line number Diff line number Diff line change
@@ -159,16 +159,24 @@ const ColumnHeaderCell = (props) => {

const columnContext = useContext(ColumnContext);
const columnHeaderCellRef = useRef();
const columnHeaderCellButtonRef = useRef();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe just call this buttonRef?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name was kept consistent with the existing columnHeaderCellRef.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I that name was used because it was the name of the component

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that columnHeaderCellButtonRef conveys more meaning than just buttonRef.


const [isResizeHandleActive, setResizeHandleActive] = useState(false);

const columnHeaderFocusArea = useCallback(() => (columnHeaderCellButtonRef.current ? columnHeaderCellButtonRef.current : columnHeaderCellRef.current), []);
Copy link
Contributor

@cm9361 cm9361 Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need a useCallback here? This should really just be a simple or statement to obtain this value, when needed. I could maybe see it being a ref variable. It just needs to be set at the proper time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on where it is called from, the useCallback is required. We did not want to do the same or statement in multiple places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just use a ref variable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is your concern about the use of a function vs a variable. There is no reason we could not have used a ref but we used a function. Does it really matter (asking for a friend ;-))?


useEffect(() => {
if (isActive && isResizeActive) {
setResizeHandleActive(true);
if (isActive) {
if (isResizable && isResizeActive) {
setResizeHandleActive(true);
} else {
columnHeaderFocusArea().focus();
setResizeHandleActive(false);
}
} else {
setResizeHandleActive(false);
}
}, [isActive, isResizeActive]);
}, [columnHeaderFocusArea, isActive, isResizable, isResizeActive]);

const onResizeHandleMouseDown = useCallback((event) => {
if (onResizeMouseDown) {
@@ -178,9 +186,9 @@ const ColumnHeaderCell = (props) => {

// Restore focus to column header after resize action is completed.
const onResizeHandleMouseUp = useCallback(() => {
columnHeaderCellRef.current.focus();
columnHeaderFocusArea().focus();
setResizeHandleActive(false);
}, []);
}, [columnHeaderFocusArea]);

// Handle column header selection via the mouse click.
const handleMouseDown = (event) => {
@@ -202,7 +210,7 @@ const ColumnHeaderCell = (props) => {
break;
case KeyCode.KEY_LEFT:
if (isResizable && isResizeHandleActive) {
columnHeaderCellRef.current.focus();
columnHeaderFocusArea().focus();
setResizeHandleActive(false);
event.stopPropagation();
event.preventDefault();
@@ -258,7 +266,10 @@ const ColumnHeaderCell = (props) => {
onKeyDown={(isSelectable || isResizable) ? handleKeyDown : null}
style={{ width: `${width}px`, height: headerHeight, left: cellLeftEdge }} // eslint-disable-line react/forbid-dom-props
>
<div className={cx('header-container')} role={displayName && 'button'}>
<div
className={cx('header-container')}
{...isSelectable && displayName && { ref: columnHeaderCellButtonRef, tabIndex: '-1', role: 'button' }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work properly for the Terra Table. In that scenario, the tabIndex would be 0. Also, should the cell tabIndex be updated when this one is set? Would the cell ever have focus in that use case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if refactoring is needed for table then it should be done at that level. We have added the requisite tests to ensure that the grid functionality is preserved if such refactoring becomes necessary. The cell has a tabIndex of -1 and I do not think that needs to change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to get away with it since the button is taking up most of the area. Ideally, the cell itself would not get focus when there is a button.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. The cell will not get focus when there is a button.

>
{errorIcon}
<span>{displayName}</span>
{sortIndicatorIcon}
Original file line number Diff line number Diff line change
@@ -269,7 +269,7 @@ describe('ColumnHeaderCell', () => {
expect(columnHeader.props().style.width).toBe('100px');
expect(columnHeader.props().style.height).toBe('150px');

const headerContainer = columnHeader.find('.header-container[role="button"]');
const headerContainer = columnHeader.find('.header-container');
expect(headerContainer.find('span').text().trim()).toBe('Vitals');
expect(headerContainer.find(IconUp)).toHaveLength(1);
expect(headerContainer.find(IconError)).toHaveLength(1);
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ exports[`ColumnHeaderCell renders a column header cell with ascending sort 1`] =
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
Vitals
@@ -64,6 +65,7 @@ exports[`ColumnHeaderCell renders a column header cell with ascending sort and e
<div
className="header-container"
role="button"
tabIndex="-1"
>
<IconError
a11yLabel="Terra.dataGrid.columnError"
@@ -113,6 +115,7 @@ exports[`ColumnHeaderCell renders a column header cell with descending sort 1`]
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
Vitals
@@ -155,6 +158,7 @@ exports[`ColumnHeaderCell renders a column header cell with error 1`] = `
<div
className="header-container"
role="button"
tabIndex="-1"
>
<IconError
a11yLabel="Terra.dataGrid.columnError"
@@ -199,6 +203,7 @@ exports[`ColumnHeaderCell renders a column header cell with onColumnSelect callb
<div
className="header-container"
role="button"
tabIndex="-1"
>
<IconError
a11yLabel="Terra.dataGrid.columnError"
@@ -247,7 +252,6 @@ exports[`ColumnHeaderCell renders a column header cell with onColumnSelect callb
>
<div
className="header-container"
role="button"
>
<IconError
a11yLabel="Terra.dataGrid.columnError"
@@ -296,6 +300,7 @@ exports[`ColumnHeaderCell renders a default column header cell 1`] = `
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
Vitals
@@ -383,6 +388,7 @@ exports[`ColumnHeaderCell renders a pinned column header cell 1`] = `
<div
className="header-container"
role="button"
tabIndex="-1"
>
<IconError
a11yLabel="Terra.dataGrid.columnError"
Original file line number Diff line number Diff line change
@@ -431,6 +431,7 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
<table
className="data-grid"
id="test-terra-data-grid"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
@@ -558,6 +559,7 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
Vitals
@@ -729,6 +731,7 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
March 16
@@ -909,7 +912,6 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
>
<div
className="header-container"
role="button"
>
<span>
March 17
@@ -2777,6 +2779,7 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
<table
className="data-grid"
id="test-terra-data-grid"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
@@ -2904,6 +2907,7 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
Vitals
@@ -3075,6 +3079,7 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
March 16
@@ -3255,7 +3260,6 @@ exports[`DataGrid verifies onCellSelect callback is triggered when space is pres
>
<div
className="header-container"
role="button"
>
<span>
March 17
@@ -5105,14 +5109,14 @@ exports[`DataGrid verifies row selection column header selection 1`] = `
<table
className="data-grid"
id="test-terra-data-grid"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
role="grid"
tabIndex={0}
>
<Memo(ColumnHeader)
activeColumnIndex={0}
activeColumnResizing={false}
columns={
Array [
@@ -5163,7 +5167,7 @@ exports[`DataGrid verifies row selection column header selection 1`] = `
columnIndex={0}
headerHeight="2.5rem"
id="WorklistDataGrid-rowSelectionColumn"
isActive={true}
isActive={false}
isResizable={false}
isResizeActive={false}
isSelectable={true}
@@ -5209,7 +5213,7 @@ exports[`DataGrid verifies row selection column header selection 1`] = `
"timeZone": null,
}
}
isActive={true}
isActive={false}
isResizable={false}
isResizeActive={false}
isSelectable={true}
@@ -5328,6 +5332,7 @@ exports[`DataGrid verifies row selection column header selection 1`] = `
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
Vitals
@@ -5499,6 +5504,7 @@ exports[`DataGrid verifies row selection column header selection 1`] = `
<div
className="header-container"
role="button"
tabIndex="-1"
>
<span>
March 16
@@ -5679,7 +5685,6 @@ exports[`DataGrid verifies row selection column header selection 1`] = `
>
<div
className="header-container"
role="button"
>
<span>
March 17
@@ -7715,6 +7720,7 @@ exports[`DataGrid verifies that the grid created is consistent with the rows and
<table
className="data-grid"
id="test-terra-data-grid"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
@@ -7733,7 +7739,6 @@ exports[`DataGrid verifies that the grid created is consistent with the rows and
}
>
<Memo(ColumnHeader)
activeColumnIndex={0}
activeColumnResizing={false}
columns={
Array [
@@ -7948,6 +7953,7 @@ exports[`DataGrid verifies the rows are created with the right props 1`] = `
<table
className="data-grid"
id="test-terra-data-grid"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
@@ -7966,7 +7972,6 @@ exports[`DataGrid verifies the rows are created with the right props 1`] = `
}
>
<Memo(ColumnHeader)
activeColumnIndex={0}
activeColumnResizing={false}
columns={
Array [
Loading