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: improving sd-header a11y #1668

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

auroraVasconcelos
Copy link
Contributor

Description:

Closes #1478 and #1519.

Definition of Reviewable:

  • Documentation is created/updated
  • Migration Guide is created/updated
  • E2E tests (features, a11y, bug fixes) are created/updated
  • Stories (features, a11y) are created/updated
  • relevant tickets are linked

@auroraVasconcelos auroraVasconcelos added the A11y Audit Identifies tasks related to accessibility improvements identified in the A11y audit of the DS label Nov 21, 2024
Copy link

github-actions bot commented Nov 21, 2024

🚀 Storybook has been deployed for branch feat_sd-header-a11y

Copy link
Contributor

@smfonseca smfonseca left a comment

Choose a reason for hiding this comment

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

After testing with VoiceOver in Safari, I feel we should have an announcement to when the drawer closes.

@@ -328,6 +328,7 @@ export default class SdDrawer extends SolidElement {
variant="tertiary"
size="lg"
part="close-button"
aria-expanded=${this.open ? 'true' : 'false'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
aria-expanded=${this.open ? 'true' : 'false'}
aria-expanded=${this.open}

@@ -11,7 +11,7 @@ export default {
},
title: 'Templates/Header'
};

/** **Accessibility hint:** aria-expanded and aria-controls may be use for smaller screens to make header accessible */
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove the "smaller screens..." part and just mention that these attributes should be set for accessibility purposes. What do you think?

@smfonseca smfonseca removed their assignment Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A11y Audit Identifies tasks related to accessibility improvements identified in the A11y audit of the DS
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

feat[dev]: ✨ implement A11y improvements to sd-header
9 participants