Make Table accessible #4629
Merged
Make Table accessible #4629
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Storybook staging is available at https://kiwicom-orbit-domi-table.surge.sh Playroom staging is available at https://kiwicom-orbit-domi-table.surge.sh/playroom |
|
Size Change: +83 B (+0.02%) Total Size: 461 kB
ℹ️ View Unchanged
|
Deploying orbit with
|
| Latest commit: |
14f7649
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://065ac890.orbit.pages.dev |
| Branch Preview URL: | https://domi-table.orbit.pages.dev |
domihustinova
force-pushed
the
domi/table
branch
2 times, most recently
from
af86f48 to
4a0d240
Compare
domihustinova
commented
Feb 25, 2025
docs/src/documentation/03-components/02-structure/table/03-accessibility.mdx
Show resolved
Hide resolved
docs/src/documentation/03-components/02-structure/table/03-accessibility.mdx
Show resolved
Hide resolved
docs/src/documentation/03-components/02-structure/table/03-accessibility.mdx
Outdated
Show resolved
Hide resolved
docs/src/documentation/05-development/04-migration-guides/01-v20.mdx
Outdated
Show resolved
Hide resolved
docs/src/documentation/05-development/04-migration-guides/01-v20.mdx
Outdated
Show resolved
Hide resolved
| @@ -137,6 +137,14 @@ intl.formatMessage({ | |||
|
|
|||
| Feel free to customize the translations according to your needs, by eventually providing more context to the modal it refers to. | |||
|
|
|||
| ## Fixes | |||
|
|
|||
| ### `scope` prop in the `TableCell` can now be set only for the `th` element (from version 20.1.0) | |||
There was a problem hiding this comment.
We need to be careful with this, as we're not 100% sure we will release 20.1.0 next. For instance:
- I have a BC in the Alert PR, which would result in 21.0.0. Not sure if it will be merged on time
- This PR has only fixes, which would result in a patch version 20.0.1
There was a problem hiding this comment.
I guess we will wait till next release to update this accordingly? 🎗️
There was a problem hiding this comment.
Let's leave it like this (we can change it to 20.0.1) and set a reminder before the release to adjust it 🤝
Edit: I set a reminder at 9:30 Thu on my side, you can do the same :)
domihustinova
force-pushed
the
domi/table
branch
2 times, most recently
from
c51d32c to
a3c6bbe
Compare
![callstackai[bot]](/~https://avatars.githubusercontent.com/in/1040204?s=60&v=4){kind=small}
| @@ -40,7 +41,7 @@ const TableCell = ({ | |||
| (align === ALIGN_OPTIONS.END || align === ALIGN_OPTIONS.RIGHT) && "text-end", | |||
| )} | |||
| data-test={dataTest} | |||
| scope={scope} | |||
| scope={props.as === TYPE_AS.TH ? props.scope : undefined} | |||
There was a problem hiding this comment.
🐛 Possible Bug
When as is set to TYPE_AS.TH, the scope attribute should be required for accessibility. Currently, the code allows table header cells without a scope attribute, which can make the table less accessible for screen readers. The scope attribute helps screen readers understand the relationship between header cells and data cells.
Suggested change
| scope={props.as === TYPE_AS.TH ? props.scope : undefined} | |
| scope={props.as === TYPE_AS.TH ? (props.scope || 'col') : undefined} |
mvidalgarcia
approved these changes
Feb 25, 2025
domihustinova
force-pushed
the
domi/table
branch
from
a3c6bbe to
14f7649
Compare
There was a problem hiding this comment.
Key Issues
Event handling changes could lead to unexpected event bubbling and unwanted parent handler triggers due to altering stopPropagation() to preventDefault(). Setting stopClickPropagation={false} on the Slide component may cause event bubbling issues as it deviates from the default behavior. Using ev.code instead of ev.keyCode might cause compatibility issues in older browsers, necessitating a fallback mechanism.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I didn't find any wrong usage of
scopein the repositories so the problem was most likely only in our stories. But to prevent wrong usage in the future, I changed the types to allowscopeonly forthelement. I added it to migration guide as a note for the users to be aware of it as advised by Marco.Closes https://kiwicom.atlassian.net/browse/FEPLT-2243
✨
Description by Callstackai
This PR enhances the accessibility of the Table component by restricting the usage of the
scopeprop to only thethelement. It also updates the documentation and migration guide accordingly.Diagrams of code changes
sequenceDiagram participant User participant TableCell participant ScreenReader Note over TableCell: Component can be rendered as "th" or "td" User->>TableCell: Render as TableHeader ("th") alt as="th" TableCell->>TableCell: Allow scope prop TableCell->>ScreenReader: Provide scope context (col/row) ScreenReader->>User: Read column/row context else as="td" TableCell->>TableCell: Scope prop not allowed end Note over User,ScreenReader: Improved accessibility for screen readersFiles Changed
scopeattribute.scopeprop to thethelement.childrenprop in the Table component documentation.TableCellcomponent to conditionally apply thescopeprop based on theasprop.Propstype to restrict thescopeprop to only be used with thethelement.This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions
.mdx,.md. See list of supported languages.