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 menu accessibility issues #257

Closed
wants to merge 5 commits into from
Closed

Fix menu accessibility issues #257

wants to merge 5 commits into from

Conversation

mollykreis
Copy link
Contributor

@mollykreis mollykreis commented Jan 12, 2022

Pull Request

🤨 Rationale

Related Bug:

👩‍💻 Implementation

This change introduces a new nimble-menu-header element to be used in place of a header in the nimble-menu. This addresses the accessibility issues flagged by storybook.

I updated the stories accordingly in storybook.

I also modified the font size & color of the Custom Menu story so that it doesn't have accessibility issues associated with it.

🧪 Testing

Viewed menu in storybook.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@mollykreis
Copy link
Contributor Author

Note: There is still an open discussion in #185 regarding the best approach to fix the issue. This is my recommendation, and since the implementation was small, I decided to move forward with the approach. If the discussion on the issue leads in a different direction, I'll update the PR accordingly.

@mollykreis mollykreis closed this Jan 19, 2022
@mollykreis
Copy link
Contributor Author

Closing this per discussion on the associated issue #185.

@mollykreis mollykreis deleted the menu-accessibility branch February 7, 2022 18:27
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