Skip to content

Conversation

@karlikpj
Copy link
Contributor

@karlikpj karlikpj commented Oct 2, 2024

Closes #1112

Pull Request Details

Add description

Closes #

Author PR Checklist

Items that the author of the PR is responsible for checking before submitted the PR.

General:

  • I have reviewed the acceptance criteria defined in the ticket and ensured the work has been completed.
  • The commit message passes all quality commit message standards.
  • Unit tests have been updated or created to reflect any javascript changes.
  • Storybook scenarios have been updated or created to reflect any html/css/js changes.

Accessibility:

  • WCAG 2.1 Level AA requirements have been met.

Development:

  • Any new or updated javascript code has 100% unit test coverage.
  • New or updated breakpoints have regression images.
  • Breaking changes have been thoroughly documented in the PR.

Product Reviewer PR Checklist

Items the product team is responsible for reviewing.

General:

  • There are no unexpected or unapproved regression image changes.

  • Functionality of interactive elements meet the acceptance criteria.
  • The product is visually and functionally the same across the different browsers.

Accessibility:

  • AxeDev Tools: there are no new or outstanding accessibility issues introduced in this PR.
  • Lighthouse: scores have not noticeably decreased during this PR.
  • Wave: there are no new errors or contrast errors introduced in this PR.

Design Reviewer PR Checklist

Items the design team is responsible for reviewing. 


General:

  • New or updated features introduced in this PR are developed mobile-first.
  • Breakpoint changes and regression images match those breakpoints.
  • This PR has been tested in all supported browsers at all breakpoints.

Developer Reviewer PR Checklist

Items the development team is responsible for reviewing.

General:

  • New code passes code quality standards set by industry standards.
  • The expected Storybook stories have been added or updated for the new or updated feature.
  • The expected unit tests have been added or updated for the new or updated feature.

Accessibility:

  • VoiceOver: Described content matches with what was expected.
  • Keyboard navigation: new or updated features and content are navigable via the keyboard.

@github-actions
Copy link

github-actions bot commented Oct 2, 2024

@karlikpj karlikpj force-pushed the ticket/1112-aria-label-guide-cards branch 3 times, most recently from d3a16e8 to c94dbdb Compare October 2, 2024 12:14
@karlikpj karlikpj marked this pull request as ready for review October 2, 2024 13:52
@karlikpj karlikpj force-pushed the ticket/1112-aria-label-guide-cards branch from c94dbdb to 9707a1b Compare October 2, 2024 13:56
@bennettcc bennettcc requested review from a team October 9, 2024 14:55
@andyvanavery31 andyvanavery31 changed the title (#1112) Add Arial Label to Guide Cards (#1112) Add Aria Label to Guide Cards Oct 9, 2024
Copy link

@andyvanavery31 andyvanavery31 left a comment

Choose a reason for hiding this comment

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

On the Doc Site, the aria-label and id not added to the Guide Cards on the Guide Card Row page.

On the Storybook, the two Variant stories, "Single Guide Card with Image and Description without Heading" and "Title Aligned Card Group", have aria-labeledby> and id> instead of aria-labeledby="guide-card-"> and id="guide-card-">. Do those need to be updated to have the same style as the other ones used across the Doc site and Storybook?

@karlikpj karlikpj force-pushed the ticket/1112-aria-label-guide-cards branch 2 times, most recently from 603efbf to 8de3896 Compare October 9, 2024 18:34
@dlescarbeau dlescarbeau requested review from dlescarbeau and removed request for dlescarbeau October 9, 2024 20:09
@bryanpizzillo
Copy link
Member

@andyvanavery31 this request would be better stated as "The list in a guide card should only be labeled by either the nci-guide-card__header or nci-guide-card__title elements."

The other elements that are being asked for use as a label are not good. They will give an accessible label that does not match the items in the list, which will be worse for accessibility.

@karlikpj karlikpj force-pushed the ticket/1112-aria-label-guide-cards branch from 8de3896 to 5c321f0 Compare October 9, 2024 20:36
@karlikpj karlikpj force-pushed the ticket/1112-aria-label-guide-cards branch from 5c321f0 to c644dc7 Compare October 9, 2024 20:43
@dlescarbeau
Copy link
Collaborator

There is a new PR up for this issue: #1927

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.

CR: Add aria-labelledby and alt-text to all Guide Cards on the NCIDS

5 participants