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.

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

Enhancement: DataTable accessibility #5839

Merged
merged 5 commits into from
Feb 1, 2024

Conversation

nitrogenous
Copy link
Contributor

@nitrogenous nitrogenous added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add Core Team Issue or pull request has been *opened* by a member of Core Team labels Jan 25, 2024
@nitrogenous nitrogenous self-assigned this Jan 25, 2024
@nitrogenous nitrogenous linked an issue Jan 25, 2024 that may be closed by this pull request
Copy link

vercel bot commented Jan 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2024 3:11pm
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2024 3:11pm

Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@@ -152,44 +164,37 @@ export const BodyRow = React.memo((props) => {
switch (event.which) {
//down arrow
case 40:
Copy link
Member

Choose a reason for hiding this comment

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

We should switch those to event.code like 'ArrowDown' etc. the old event.which is deprecated

@@ -169,7 +169,7 @@ export const HeaderCell = React.memo((props) => {
};

const onKeyDown = (event) => {
if (event.key === 'Enter' && event.currentTarget === elementRef.current && DomHandler.getAttribute(event.currentTarget, 'data-p-sortable-column') === 'true') {
if ((event.which == 13 || event.which == 32) && event.currentTarget === elementRef.current && DomHandler.getAttribute(event.currentTarget, 'data-p-sortable-column') === 'true') {
onClick(event);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to use event.which anymore it should all be event.code right?

Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@mertsincan mertsincan merged commit 348bd93 into master Feb 1, 2024
5 of 6 checks passed
@mertsincan mertsincan deleted the 5799-datatable-accessibility-support branch February 1, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Team Issue or pull request has been *opened* by a member of Core Team Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataTable Accessibility Support
4 participants