-
Notifications
You must be signed in to change notification settings - Fork 4.2k
AI Rename UI: fix highlighted item on focus #81445
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
AI Rename UI: fix highlighted item on focus #81445
Conversation
|
After def looks good. I have no idea how the UI framework works though. @JoeRobich can you ptal? |
| // update the ComboBox selection to match the focused item | ||
| if (e.NewFocus is ComboBoxItem comboBoxItem && comboBoxItem.DataContext != null) | ||
| { | ||
| SelectedItem = comboBoxItem.DataContext; |
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.
why is this needed?
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.
Seems like we wouldn't want to select the item until the user hits enter or space.
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.
Joey, "Seems like we wouldn't want to select the item until the user hits enter or space." is not consistent with current behavior.
Without this event handler, the highlighted item does not get previewed. Then, upon pressing Enter, it's not going to be committed (because it's not inserted into the TextBox)

That's not how we currently handle up/down, where the item is selected, inserted into the TextBox, and ultimately used for rename on Enter.

The above code makes the TAB selection behave the same way as Up/Down selection ^
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.
Ah, sounds good. Thanks @AmadeusW!
| // update the ComboBox selection to match the focused item | ||
| if (e.NewFocus is ComboBoxItem comboBoxItem && comboBoxItem.DataContext != null) | ||
| { | ||
| SelectedItem = comboBoxItem.DataContext; |
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.
Seems like we wouldn't want to select the item until the user hits enter or space.
We have an accessibility bug (azdo 2615738) for the Smart Rename UI, where the focus indicator is hard to see when user set keyboard focus on an item using Tab, instead of the typical Up/Down that works correctly.
Before: (note focus is on the "DashboardController" item)

After:
