-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(menu): Added new API to manually set focus when menu is opened #4468
Conversation
I think the Don't forget to update the commit message before you hit "Squash and merge" 😀 |
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.
Thanks for the quick review!
Codecov Report
@@ Coverage Diff @@
## master #4468 +/- ##
=========================================
Coverage ? 99.02%
=========================================
Files ? 130
Lines ? 6277
Branches ? 820
=========================================
Hits ? 6216
Misses ? 60
Partials ? 1
Continue to review full report at Codecov.
|
Ping. |
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.
When I test the baseline demo page, for some reason pressing space on the button does nothing, but I'd expect it to act the same as clicking with the mouse. It also seems to lose focus when I do that?
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.
Updated menu/fixture.js
to handle space key in screenshot test demo. Thanks for spotting it out.
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.
In the BREAKING CHANGE:
note, include info on the call required to match the old default behavior.
Andy's review comments are resolved
🤖 Beep boop! Screenshot test report 🚦9 screenshots changed from |
Fixes #3922
Superseded by changes in PR #4587
BREAKING CHANGE: Focus is no more set to first menu item when menu is opened. Introduced new API (
setDefaultFocusItemIndex()
) to set focus on specific menu item every time the menu is opened. Also introduced new foundation & adapter methods to incorporate this change. Please usesetDefaultFocusItemIndex(0)
method before menu open to retain previous behaviour.