Skip to content
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.

Sign up for GitHub

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

chore(Accessibility): Fix accessibility for 'Show x entries' dropdown in tables #30818

Merged
merged 8 commits into from
Nov 18, 2024
Merged

chore(Accessibility): Fix accessibility for 'Show x entries' dropdown in tables #30818

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9425fdf
chore:Fix accessibility for 'Show x entries' dropdown in tables
LevisNgigi Nov 1, 2024
53d88f5
Fix linting issues in superset-frontend
LevisNgigi Nov 6, 2024
647fb9d
fix:add Show x entry per page label
LevisNgigi Nov 7, 2024
a1d6708
fix:package-lock.json
LevisNgigi Nov 7, 2024
bf3137a
fix:npm dependancies
LevisNgigi Nov 8, 2024
255c6b8
fix:package-lock.json
LevisNgigi Nov 8, 2024
c78e9ac
Merge branch 'apache:master' into fix-accessibility-show-entries
LevisNgigi Nov 8, 2024
049eb75
Fixed accessibility issue for the dropdown in Firefox
LevisNgigi Nov 13, 2024
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
23 changes: 13 additions & 10 deletions superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,33 +197,36 @@ function SelectPageSize({
onChange,
}: SelectPageSizeRendererProps) {
return (
<span className="dt-select-page-size form-inline">
{t('page_size.show')}{' '}
<span
className="dt-select-page-size form-inline"
role="group"
aria-label={t('Select page size')}
>
<label htmlFor="pageSizeSelect" className="sr-only">
{t('Select page size')}
</label>
{t('Show')}{' '}
<select
id="pageSizeSelect"
className="form-control input-sm"
value={current}
onBlur={() => {}}
onChange={e => {
onChange(Number((e.target as HTMLSelectElement).value));
}}
aria-label={t('Show entries per page')}
Copy link
Member

@geido geido Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think it should be enough to have just this aria-label here and set the number of entries here (while removing the aria-label from the individual options). Otherwise, the screen reader is reading "Show 10 entries per page, Show entries per page" which I think it is unnecessary. Also, on Firefox, when inspecting for accessibility I am getting this:

Screenshot 2024-11-13 at 16 09 29

I believe we need to have a label that labels the Select input. We can probably change the structure a bit so to keep the same layout but have the proper Label element and accessibility properties. What do you think?

>
{options.map(option => {
const [size, text] = Array.isArray(option)
? option
: [option, option];
const sizeLabel = size === 0 ? t('all') : size;
return (
<option
aria-label={t('Show %s entries', sizeLabel)}
key={size}
value={size}
>
<option key={size} value={size}>
{text}
</option>
);
})}
</select>{' '}
{t('page_size.entries')}
{t('entries per page')}
</span>
);
}
Expand Down
Loading