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

fix(TableHead): add id prop on TableExpandHeader #3611

Merged
merged 3 commits into from
Oct 24, 2022
Merged

Conversation

jessieyan
Copy link
Collaborator

@jessieyan jessieyan commented Oct 19, 2022

Closes #3485

Summary

  • fix the a11y violation by adding id prop on TableExpandHeader

According to the rule H43: Using id and headers attributes to associate data cells with header cells in data tables

Header should use id prop for the any table cell to reference with. The default header for the first cell in TableExpandRow is set to headers='expand'. But we did not set an id on TableExpandHeader for the cell to refer to.

The solution is to set id={${tableId}-expand} on TableExpandHeader and expandHeader={${tableId}-expand} on TableExpandRow:

Change List (commits, features, bugs, etc)

-fix(TableBodyRow): add id prop on TableExpandHeader

Acceptance Test (how to verify the PR)

  • go to table story with row expansion
  • scan with IBM Equal Access Accessibility
  • check no errors shows like 'The 'headers' attribute value "expand" does not reference a valid 'id' in this document'

before:
image

after:
image

Regression Test (how to make sure this PR doesn't break old functionality)

  • tests here

Things to look for during review

  • Make sure all references to iot or bx class prefix is using the prefix variable
  • (React) All major areas have a data-testid attribute. New test ids should have test written to ensure they are not changed or removed.
  • UI should be checked in RTL mode to ensure the proper handling of layout and text.
  • All strings should be translatable.
  • The code should pass a11y scans (The storybook a11y knob should show no violations). New components should have a11y test written.
  • Unit test should be written and should have a coverage of 90% or higher in all areas.
  • All components should be passing visual regression test. For new styles or components either a visual regression test should be written for all permutations or the base image updated.
  • Changes or new components should either write new or update existing documentation.
  • PR should link and close out an existing issue

@jessieyan jessieyan requested a review from davidicus as a code owner October 19, 2022 20:21
@netlify
Copy link

netlify bot commented Oct 19, 2022

Deploy Preview for carbon-addons-iot-react ready!

Name Link
🔨 Latest commit dee0373
🔍 Latest deploy log https://app.netlify.com/sites/carbon-addons-iot-react/deploys/6351840c8e19540008064899
😎 Deploy Preview https://deploy-preview-3611--carbon-addons-iot-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 site settings.

@netlify
Copy link

netlify bot commented Oct 19, 2022

Deploy Preview for ai-apps-pal-angular ready!

Name Link
🔨 Latest commit dee0373
🔍 Latest deploy log https://app.netlify.com/sites/ai-apps-pal-angular/deploys/6351840cac89eb0009c4a251
😎 Deploy Preview https://deploy-preview-3611--ai-apps-pal-angular.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 site settings.

@jessieyan jessieyan changed the title fix(TableBodyRow): add expandHeader prop on TableExpandRow as the id fix(TableBodyRow): add id prop on TableExpandHeader Oct 19, 2022
@jessieyan jessieyan changed the title fix(TableBodyRow): add id prop on TableExpandHeader fix(TableHead): add id prop on TableExpandHeader Oct 19, 2022
Copy link
Collaborator

@herleraja herleraja left a comment

Choose a reason for hiding this comment

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

LGTM

@jessieyan
Copy link
Collaborator Author

Brought it to scrum today. We will use ${tableId}-expand for the expandHeader on TableExpandRow and id on TableExpandHeader.

@kodiakhq kodiakhq bot merged commit c10eb79 into next Oct 24, 2022
@kodiakhq kodiakhq bot deleted the fix-3485-expandHeader branch October 24, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Table] [A11y] headers="expand" should be removed from first column <td>
3 participants