Skip to content

chore(Table): remove tabindex setting code #19414

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wkeese
Copy link
Contributor

@wkeese wkeese commented May 17, 2025

Refs IBMa/equal-access#2281.

As Chrome and Firefox now both support keyboard scrolling on divs, we no longer need to set tabindex on scrollable nodes, even if they don't contain any interactive content.

So remove the complicated Table code that tries to do that.

See carbon-design-system/ibm-products#7043 (comment).

Changelog

Removed

  • Code for setting tabindex

Testing / Reviewing

You can test this on
http://localhost:3000/?path=/story/components-datatable-basic--default. If you make your browser narrow enough, the table gets a horizontal scrollbar, and you can tab to it.

PR Checklist

As the author of this PR, before marking ready for review, confirm you:

  • Reviewed every line of the diff
  • Updated documentation and storybook examples
  • Wrote passing tests that cover this change
  • Addressed any impact on accessibility (a11y)
  • Tested for cross-browser consistency
  • Validated that this code is ready for review and status checks should pass

More details can be found in the pull request guide

We know longer need to set tabindex on scrollable nodes, even if they
don't contain any interactive content.  So remove the complicated
Table code that tries to do that.

You can test this on
http://localhost:3000/?path=/story/components-datatable-basic--default.
If you make your browser narrow enough, the table gets a horizontal
scrollbar, and you can tab to it.

See carbon-design-system/ibm-products#7043 (comment).

Refs carbon-design-system#7043.
Copy link

netlify bot commented May 17, 2025

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit b087e5c
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/6827dcc99e2dbe0008226043
😎 Deploy Preview https://deploy-preview-19414--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

netlify bot commented May 17, 2025

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b087e5c
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/6827dcc956714c000823d02e
😎 Deploy Preview https://deploy-preview-19414--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

codecov bot commented May 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.84%. Comparing base (0ce2c4e) to head (b087e5c).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #19414      +/-   ##
==========================================
- Coverage   84.85%   84.84%   -0.01%     
==========================================
  Files         371      371              
  Lines       14412    14400      -12     
  Branches     4746     4744       -2     
==========================================
- Hits        12229    12218      -11     
+ Misses       2036     2035       -1     
  Partials      147      147              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wkeese wkeese marked this pull request as ready for review May 17, 2025 01:34
@wkeese wkeese requested review from a team as code owners May 17, 2025 01:34
@wkeese wkeese requested review from preetibansalui and annawen1 May 17, 2025 01:34
@wkeese
Copy link
Contributor Author

wkeese commented May 19, 2025

I'll mark this as a draft though until there's an official decision on what Carbon wants to do. I filed IBMa/equal-access#2281 to talk about removing the requirement from accessibility-checker, and then we can decide on Carbon after that.

@wkeese wkeese marked this pull request as draft May 19, 2025 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant