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

Do we need "Voice" as part of the voicing combo box interaction in Preferences menu? #813

Closed
terracoda opened this issue Jun 2, 2022 · 13 comments

Comments

@terracoda
Copy link
Contributor

terracoda commented Jun 2, 2022

I originally designed a dynamic button name "Voice {{Alex}}" for the preferences menu.

The button lives inside an accordion box with the name "Customize Voice", and then there are two sliders before the combo box:

Customize Voice (voiced as "Customize Voice" + context responses when expanded/collapsed)
"Rate" (voiced as "Voice Rate" + actual rate) - Voice Rate, {{1.75}} times
"Pitch" (voiced as "Pitch" + actual range) - Pitch, {{in normal range}}
"{{Alex}}" (selected voice voiced on initial focus, but new voice is not voiced when a new voice is selected.)

I just want to double check that we intentionally do not want "Voice" as part of the button's visual label or accessible name, and that we do not want to hear the newly selected voice when a change is made.

I'd like to note that while working on RaP's two combo boxes, we came up with 2 distinct combo box voicing interaction patterns, one for when then the button label incorporates the name "Challenge {{1}}" and one where the the visual label is outside "Range:" + "{{0 to 10}}".

I am wondering if one of these patterns would be a better fit for the Voice combo box?
And it would be great to have a little discussion about this issue before we publish our next sim with Voicing.

@jessegreenberg
Copy link
Contributor

I am fine with whatever is decided, though adding a "Voice" label makes the control stick out a bit farther (it can get pretty wide for the microsoft voices).

image

@jessegreenberg jessegreenberg removed their assignment Jun 2, 2022
@zepumph
Copy link
Member

zepumph commented Jun 2, 2022

Can the "Voice" label be above it?

@brettfiedler
Copy link
Member

I'm fine with it being above. Will the Pref menu get longer or will the text need to be resized?
image

@jessegreenberg
Copy link
Contributor

Can the "Voice" label be above it?

Oh yes, I suppose so. Though changing layout of the label isn't currently a supported option of ComboBox.

@zepumph
Copy link
Member

zepumph commented Jun 6, 2022

@terracoda, it would be a bit challenging to add the label above the combobox. Can we just keep it the way it is now?

@zepumph zepumph removed their assignment Jun 6, 2022
@terracoda
Copy link
Contributor Author

terracoda commented Jun 8, 2022

Can the "Voice:" part of the name be hidden? We already have "Voice Pitch" and only "Pitch" is on the screen.

@terracoda terracoda assigned zepumph and unassigned terracoda and brettfiedler Jun 8, 2022
@terracoda
Copy link
Contributor Author

terracoda commented Jun 8, 2022

I would think that the design pattern would be the same as RaP's "Range" combobox.
But with the "Range:" part hidden.

@terracoda
Copy link
Contributor Author

If that approach is too complicated. We can leave it.

@zepumph
Copy link
Member

zepumph commented Jun 8, 2022

Ahh I see, perhaps something like this:

Index: js/preferences/VoicingPanelSection.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/preferences/VoicingPanelSection.js b/js/preferences/VoicingPanelSection.js
--- a/js/preferences/VoicingPanelSection.js	(revision e445764c07dc704f7bd47ddaae091e6d6b245de0)
+++ b/js/preferences/VoicingPanelSection.js	(date 1654701947879)
@@ -431,6 +431,8 @@
       listPosition: 'above',
       accessibleName: voiceLabelString,
 
+      comboBoxVoicingNameResponsePattern: 'Voice: {{value}}',
+
       // phet-io, opt out because we would need to instrument voices, but those could change between runtimes.
       tandem: Tandem.OPT_OUT
     }, options );

@zepumph
Copy link
Member

zepumph commented Jun 8, 2022

Ok how does that sound on master?

zepumph added a commit that referenced this issue Jun 8, 2022
@terracoda
Copy link
Contributor Author

terracoda commented Jun 8, 2022

I think this sounds better. I hear "Voice {{selectedVoice}}" when I click on or move focus to the combobox button. I also hear each voice option as I arrow through the options.

However, my selection is not voiced. And that's how it woks with the Range combobox as well.

I checked the RaP design doc to see if we had a good reason for not hearing the new selection, and Rule 2 says

  1. Upon a new selection, the list of options closes, the new option becomes the visible name on the button, and the current context for the hands changes. Voice name response and context response according to checked preferences.

@zepumph, is there a reason why we did not implement the name response upon a new selection? Or did I just miss this in my last review of the combobox interaction? Or did something change in common code?

There is no context response for the Voice combobox, so we won't hear anything there even if checked. We should always hear a name response, though, according to my design documentation.

I am wondering if the missing name response that should happen upon a selection is a bug or an undocumented design decision?

I think it is a bug, so can you add a name response or show me the issue where we decided not to have name responses upon a new selection and I will update the design documentation.

This stuff is complicated!

@terracoda
Copy link
Contributor Author

"Voice" is in, so this particular issue is resolved. Shall we move my last comment to a new issue - a common code issue?

@zepumph
Copy link
Member

zepumph commented Jun 9, 2022

Let's take up #813 (comment) in phetsims/ratio-and-proportion#474.

@zepumph zepumph closed this as completed Jun 9, 2022
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

4 participants