-
Notifications
You must be signed in to change notification settings - Fork 126
feat(core): refactor keyboard navigation #2268
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: d61eaaa The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @danielleroux, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant overhaul of keyboard navigation and focus management for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the keyboard navigation for dropdown-related components to align with W3C accessibility patterns. It introduces a new focus management system using an ix-focusable class and a global listener to apply ix-focused styles, replacing the previous ArrowFocusController. A new showChange event is added to dropdowns to allow cancellation before showing. The changes are extensive and touch upon core components, their styling, and framework-specific wrappers. My review focuses on potential issues like the use of !important in styles, an empty catch block that could hide errors, leftover debug code, and unclear documentation.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant refactoring of keyboard navigation and focus management, primarily for dropdown-related components, to align with W3C accessibility patterns. A custom focus-visible listener has been implemented to provide more control over focus styling, moving away from native :focus-visible. This is a substantial and positive change for accessibility. The refactoring touches many components, including dropdown, select, button, and avatar, with new properties and events to support the new focus model. The tests have been updated to reflect these changes. I've identified one critical issue and a couple of medium-severity issues that should be addressed.
|



💡 What is the current behavior?
GitHub Issue Number: #
🆕 What is the new behavior?
Refactor keyboard navigation to match the common w3 accessibility patterns for
ix-dropdownrelated components🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support