Skip to content

fix(data-grid-cell): add button role announcement for screen readers (VIV-2606) #2325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion libs/components/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1904,7 +1904,7 @@
},
{
"name": "gridTemplateColumns",
"description": "String that gets applied to the the css gridTemplateColumns attribute of child rows",
"description": "String that gets applied to the css gridTemplateColumns attribute of child rows",
"type": "string",
"attributeName": "grid-template-columns",
"propertyName": "gridTemplateColumns"
Expand Down
8 changes: 8 additions & 0 deletions libs/components/src/lib/data-grid/data-grid-cell.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,14 @@ describe('vwc-data-grid-cell', () => {
element.ariaSort = 'none';
});

it('should have a button role when sorting is enabled', async function () {
element.setAttribute('aria-sort', 'none');
await elementUpdated(element);
const baseElement = element.shadowRoot?.querySelector('.base');

expect(baseElement?.role).toEqual('button');
});

it('should show sort-solid icon in the header when "none" is set', async function () {
element.setAttribute('aria-sort', 'none');
await elementUpdated(element);
Expand Down
5 changes: 4 additions & 1 deletion libs/components/src/lib/data-grid/data-grid-cell.template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ export const DataGridCellTemplate = (
@click="${(x) => x._handleInteraction()}"
@keydown="${(x, c) => handleKeyDown(x, c.event as KeyboardEvent)}"
>
<div class="base">
<div
class="base"
role="${(x) => (shouldShowSortIcons(x) ? 'button' : undefined)}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that is what we want, because the the whole header cell becomes a button now
So maybe the role should be just on the sort icon and it should have a label of "Sort"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took my inspiration from these two sources:

  1. https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/data-grids/
  2. https://mulder21c.github.io/aria-practices/examples/table/sortable-table.html

In both of these examples the header cell element got its aria-sort attributes, and their children are either native buttons, or roles set to button.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mhm, in our data grid you could theoretically have buttons inside the header as well (like filter), which would be inside the button now.
Some data-grids have sort at a separate button like that: https://mui.com/x/react-data-grid/sorting/
But I'm not sure what the best way is, so I'll approve it because at least it's better than before

Choose a reason for hiding this comment

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

Awesome, thanks :).

Also - I understand your point. Thanks for the example.

I've tried it first, but it felt kinda unnatural for me, to use the tab button to reach the sorting feature, while using roving tabindex for the rest of the table.

I found this solution to be the cleanest, without adding "use tab key to use sorting button" type labels and descriptions and then dealing with i18n.

>
<slot></slot>
${(_) => renderSortIcons(context)}
</div>
Expand Down
2 changes: 1 addition & 1 deletion libs/components/src/lib/data-grid/data-grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export class DataGrid extends VividElement {
}

/**
* String that gets applied to the the css gridTemplateColumns attribute of child rows
* String that gets applied to the css gridTemplateColumns attribute of child rows
*
* @public
* @remarks
Expand Down
Loading