feat: Autolabel table and cards with header #1364
Conversation
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
22baa33
to
8e2d096
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1364 +/- ##
=======================================
Coverage 93.58% 93.59%
=======================================
Files 636 637 +1
Lines 16934 16950 +16
Branches 5581 5584 +3
=======================================
+ Hits 15848 15864 +16
Misses 1014 1014
Partials 72 72
☔ View full report in Codecov by Sentry. |
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
a5c1d7c
to
27fafbc
Compare
src/box/internal.tsx
Outdated
| @@ -24,7 +25,12 @@ export default function InternalBox({ | |||
| __internalRootRef = null, | |||
| ...props | |||
| }: InternalBoxProps) { | |||
| const baseProps = getBaseProps(props); | |||
| const wrapperHeadingId = useContext(ScrollbarLabelContext); | |||
There was a problem hiding this comment.
Box is one of our most used components in the system. I'm a little scared of adding a React context to it. Have you checked this for potential performance degredation?
There was a problem hiding this comment.
Box is definitely tricky, and there are more concerns (like pointing aria to a non-existing id). So I changed the implementation to provide an id from the header component.
But it might still worth checking for performance issues in a dev pipeline first
| // SPDX-License-Identifier: Apache-2.0 | ||
| import { createContext } from 'react'; | ||
|
|
||
| export const ScrollbarLabelContext = createContext<string | undefined>(undefined); |
There was a problem hiding this comment.
Why is this called ScrollbarLabelContext? It sounds like we're adding a label to a scrollbar.
There was a problem hiding this comment.
Naming is difficult! We are adding a label to table / cards kinda because of a scrollbar. But maybe, CollectionLabelContext sounds better
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
27fafbc
to
6bb84e0
Compare
| @@ -12478,12 +12478,12 @@ scroll parent scrolls to reveal the first row of the table.", | |||
| * \`allItemsSelectionLabel\` ((SelectionState) => string) - Specifies the alternative text for multi-selection column header. | |||
| * \`selectionGroupLabel\` (string) - Specifies the alternative text for the whole selection and single-selection column header. | |||
| It is prefixed to \`itemSelectionLabel\` and \`allItemsSelectionLabel\` when they are set. | |||
| * \`tableLabel\` (string) - Provides an alternative text for the table. If you use a header for this table, you may reuse the string | |||
There was a problem hiding this comment.
feels like this property was placed above the following text by accident
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
6bb84e0
to
cad818e
Compare
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
cad818e
to
63a8da4
Compare
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
a09400c
to
b9d77f3
Compare
Al-Dani
force-pushed
the
table-cards-auto-label
branch
from
b9d77f3
to
fee5108
Compare
Description
Scrollable tables and cards create an unlabelled tabIndex, which leads to screen readers reading out all the contents. This can be fixed manually by providing
ariaLabels.tableLabel(cardsLabel)property, but it is not commonly used. To simplify this, we will automatically label table (cards) with a header.Because header is a slot, we will use context to provide id to the
<Header>component, assuming that<Header>is the recommended way to entitle collections.For the custom headers it's still required to provide
ariaLabels.tableLabel(cardsLabel)propertyRelated links, issue #, if available: AWSUI-21380
How has this been tested?
New unit tests are added, tested with VO+Safari
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.