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

Add hidden context for icons in rows for summary tables #3137

Closed
wants to merge 14 commits into from

Conversation

adi-unni
Copy link
Contributor

@adi-unni adi-unni commented Apr 11, 2024

What is the context of this PR?

Fixes: #3030

This PR fixes the lack of context in summary tables where there is no status column and only an icon which in the case of the ticket, were checks for completion. Following from the DAC advice, we are now adding a visually hidden <span> to the icon's <div> to enable adding additional contextual information. This done by setting the "iconVisuallyHiddenText": {string} param in rowItems adjacent to "IconType": {icon}. The service can input the appropriate text to add context to whichever icon is being used.

Please make sure that this param is set when there is no other context on the row indicating section completion status (see the new minimal example).

How to review this PR

Check the new example-minimal for the summary table and see if the new span tag with section completed is there. Sense check the output from the screen reader

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

Copy link

netlify bot commented Apr 11, 2024

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit 7de10ec
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/662f657163ca9f000870af02
😎 Deploy Preview https://deploy-preview-3137--ons-design-system-preview.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 configuration.

@adi-unni adi-unni added the Accessibility Issues discovered through accessibility testing label Apr 15, 2024
@adi-unni adi-unni requested a review from a team April 15, 2024 10:19
@adi-unni adi-unni marked this pull request as ready for review April 15, 2024 10:20
@adi-unni adi-unni changed the title Add context for icons in rows Add hidden context for icons in rows Apr 15, 2024
@adi-unni adi-unni changed the title Add hidden context for icons in rows Add hidden context for icons in rows for summary tables Apr 15, 2024
@rmccar
Copy link
Contributor

rmccar commented Apr 16, 2024

Im a bit confused as to why the VR snapshots have been updated on this PR for the change to the width of radios that has just been merged in another PR. They should've been updated on the other PR. Ive already spoken to Alessio, we may need to look into our process of updating the snapshots on PRs and this could be the reason why we have been having problems with backstopJS recently.

@adi-unni
Copy link
Contributor Author

Cant fix the VR tests so have opened a new PR #3165

@adi-unni adi-unni closed this Apr 30, 2024
@alessioventuriniAND alessioventuriniAND deleted the enhancement/3030/context-for-icons branch July 15, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Issues discovered through accessibility testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants