-
Notifications
You must be signed in to change notification settings - Fork 1
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
Make highlight Actions menu items (Edit and Delete) buttons #2332
base: main
Are you sure you want to change the base?
Conversation
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.
As I read the comment, the focus concern is that the "dialog" (the DisplayNote) doesn't get focus when it appears. That is by design; it's what alt-H is for. |
f0295ce
to
d76dda2
Compare
d76dda2
to
7375997
Compare
I'm going to have it move focus to the first item. |
f39e405
to
ce568dc
Compare
@jivey Focusing on the first item can result in the hover-highlight competing with the focus-highlight, so I had to add code to make focus follow the mouse within the menu. It's ready to 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.
The styles look good, but with the hover change, it can get into a weird state where one item is highlighted by the mouse and the other the keyboard:
I'm not sure how react-aria-component's Menu handles it internally, but that seems like a good interaction goal if we can make it work like that.
86411dc
to
8c24ab4
Compare
I resolved the focus/hover issue by styling focus and not hover. |
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.
Looks good and feels good to use, thanks for going through those review rounds with me.
32486b2
to
e4055ab
Compare
c1d0a5f
to
4b0421c
Compare
e0feeaa
to
dd06dbe
Compare
df0b54f
to
5ca988a
Compare
DISCO-307
For clarity, changed
ol
tomenu
Also simplified return value of DropdownItem.