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

ComboBox voicing response on selection broken #474

Closed
zepumph opened this issue Jun 8, 2022 · 6 comments
Closed

ComboBox voicing response on selection broken #474

zepumph opened this issue Jun 8, 2022 · 6 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Jun 8, 2022

I can reproduce this in the voice combobox in the voicing preferences, and also in the my challenge combo box. I'll take a look. I believe this is a regression, though it could have been part of the design. I thought it should say the selection when you press it. This is broken for mouse and keyboard.

@zepumph zepumph added type:bug Something isn't working type:question Further information is requested labels Jun 8, 2022
@zepumph zepumph self-assigned this Jun 8, 2022
@zepumph
Copy link
Member Author

zepumph commented Jun 8, 2022

Discovered while poking around phetsims/joist#813

@zepumph
Copy link
Member Author

zepumph commented Jun 10, 2022

Ahhh, @jessegreenberg, we have broken voicing responses on combobox selection because of voicingVisible. The listbox get's hidden, so we don't hear it even though the combobox code hasn't changed.

https://github.com/phetsims/sun/blob/811a46cb8e395efbbe0e59dd603af474c5d5d045/js/ComboBoxListBox.ts#L311-L326

I'll investigate a way to voice through a visible part of the combobox.

@zepumph
Copy link
Member Author

zepumph commented Jun 10, 2022

@terracoda please note here, feel free to test if you'd like and let me know if you run into trouble.

@jessegreenberg, will you give me a spot check here?

@terracoda
Copy link
Contributor

terracoda commented Jun 16, 2022

The Voice combobox sounds good on open and upon selection of new voice.
Oh, this is assigned to @jessegreenberg to verify.

@zepumph
Copy link
Member Author

zepumph commented Jun 22, 2022

Looks like I missed the contextResponse as part of this issue. Fixed in phetsims/sun@ec1d8fd.

@jessegreenberg
Copy link
Contributor

Great, this is working well thank you for fixing this.

I thought briefly about moving this into ComboBoxButton somehow so that we didn't have to add to the ComboBoxListBox constructor args. But I think this needs to happen in the ComboBoxListBox FireListener.

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

3 participants