fix(aria/combobox): popup not closing when focus leaves it - #33697
Conversation
15c903a to
60ae18f
Compare
ok7sai
left a comment
ok7sai
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Overall looks great with some comments.
| /** Handles focus out events for the combobox. */ | ||
| onFocusout() { | ||
| this.closePopupOnFocusout(); | ||
| this.isFocused.set(false); |
There was a problem hiding this comment.
Let's keep it consistent with ComboboxPopupPattern that sets the isFocused to false before calling the closePopupOnFocusout.
this.isFocused.set(false);
this.closePopupOnFocusout();
There was a problem hiding this comment.
Good catch, done. closePopupOnFocusout reads isFocused inside a deferred setTimeout, so the order between the two calls never changed behavior, but matching ComboboxPopupPattern is worth it for consistency. Fixed in c0739c1.
There was a problem hiding this comment.
Let's also add tests to src/aria/combobox/combobox.spec.ts for testing directives in real use cases.
There was a problem hiding this comment.
Added two tests in src/aria/combobox/combobox.spec.ts under ComboboxWidget, using the real Combobox/ComboboxPopup/ComboboxWidget directives and a dialog popup with a focusable child. One confirms the popup closes when focus leaves it entirely without ever returning to the trigger, the other confirms it stays open when focus moves back to the trigger. Verified the first test fails on the pre-fix code and passes with it. Done in c0739c1.
ok7sai
added
action: merge
* fix(aria/combobox): popup not closing when focus leaves it * fix(aria/combobox): set isFocused before closing popup on focusout (cherry picked from commit de579cc)
2 participants
Fixes that the popup was only closed from the trigger's
focusouthandler. When the popup renders outside the trigger and takes focus, the trigger never blurs again, so clicking away left the popup open.ComboboxPopupPattern.onFocusoutnow runs the same check once focus has left the popup as well.Fixes #33696.