Skip to content
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

Closing a ComboBoxListBox should restore focus to ComboBoxButton before changing Property value. #721

Closed
pixelzoom opened this issue Oct 4, 2021 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Oct 4, 2021

Over in phetsims/fourier-making-waves#194, I ran into a case where making a ComboBox selection results in a Dialog opening (to alert the user to an invalid combination of UI settings). When closing that Dialog, I discovered that focus was not restored to the ComboBoxButton. @jessegreenberg pointed out that the focus had actually been on a ComboBoxListItemNode in the ComboBoxListBox, and we couldn't restore focus to it since the listbox was popped down.

I noted that selecting from a ComboBoxListBox typically restores focus to the ComboBoxButton, and wondered why that wasn't happening here. We discovered that it is due to this bit of code in ComboBoxListBox:

    // Pops down the list box and sets the property.value to match the chosen item.
    const fireAction = new Action( event => {
...
      // set value based on which item was chosen in the list box
      property.value = listItemNode.item.value;
...
      focusButtonCallback();
    } , 
...

My dialog opens in response to the Property value being changed. Because the Property is changed before restoring focus to the ComboBoxButton, the dialog cannot restore focus when it's closed. The solution is to change the order of these 2 operations.

@pixelzoom
Copy link
Contributor Author

Fixed in the above commits, in master and fourier-making-waves-1.0 branches.

@jessegreenberg would you please review?

@jessegreenberg
Copy link
Contributor

I think this change makes sense and fixes the problem found in phetsims/fourier-making-waves#194. I tested a number of other ComboBoxes just in case but I can't think of anything problematic with this change.

pixelzoom added a commit that referenced this issue Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants