-
Notifications
You must be signed in to change notification settings - Fork 70
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
combobox select on blur tabToSelect #3399
base: main
Are you sure you want to change the base?
combobox select on blur tabToSelect #3399
Conversation
|
@ThusharaJ07 is attempting to deploy a commit to the FED Team Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -248,7 +249,7 @@ export const ComboBox = forwardRef(function ComboBox<Item>( | |||
|
|||
break; | |||
case "Tab": | |||
if (!multiselect && activeState) { | |||
if (tabToSelect && activeState) { |
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.
this is a breaking change as the single select menu used to select on TAB and now it only will, if you provide this prop
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.
@mark-tate @origami-z yes, now with this prop only we will be able to select the typed item on tab key press. the behaviour is kept common for both single/multiselect combobox for consistency.
Let me know if we want to only apply this for multiselect only.
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.
From meeting: let's try default to different values in single / multi select
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.
@mark-tate @origami-z Added the below change to not break the existing behaviour >>>
the default value for prop will be:
- true for single select
- false for multiselect
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.
Makes sense. Let's add some test for both
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.
@origami-z have added test.
@@ -16,6 +16,7 @@ data: | |||
- If the combo box is single-select and if focus is above the combo box, Tab sets focus on the component. When focused, combo box has a visible text cursor and the list is opened. | |||
- If the combo box is multi-select and if the focus is above the combo box, Tab sets focus on the pill list if pills are present. If no pills are present then focus is set on the input. | |||
- If the list is open, Tab moves focus to the next focusable element in the tab order and selects the current active option. |
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.
Could combine this with next point?
…comobox-select-on-blur
#3484