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

Add keyboard help information for interaction with the flux meter #180

Closed
Tracked by #885
jbphet opened this issue Jun 15, 2022 · 10 comments
Closed
Tracked by #885

Add keyboard help information for interaction with the flux meter #180

jbphet opened this issue Jun 15, 2022 · 10 comments

Comments

@jbphet
Copy link
Contributor

jbphet commented Jun 15, 2022

Interaction with the flux meter will need some custom explanations in the Keyboard Help dialog. This should be designed first, then implemented.

Assigning to @arouinfar for the design part.

@arouinfar
Copy link
Contributor

The current plan is to implement the probe as an accessible slider. In that case, we may not need specific content for the Energy Flux Meter.

@arouinfar
Copy link
Contributor

Tagging for design meeting. The current implementation uses a slider, so I don't think we need specific information in the keyboard help dialog, but let's confirm with the team.

@jbphet
Copy link
Contributor Author

jbphet commented Sep 21, 2022

At today's meeting we decided that we would change the heading for the Slider Controls in the keyboard help dialog to say "Slider and Flux Meter" controls. This pertains to the screens that include a flux meter, which are the Photons and Layer Model screens.

@arouinfar arouinfar removed their assignment Sep 26, 2022
@jbphet jbphet self-assigned this Dec 14, 2022
@jbphet
Copy link
Contributor Author

jbphet commented Dec 20, 2022

@arouinfar - I just did some investigation into changing the keyboard help dialog section that says "Slider Controls" to "Slider and Flux Meter Controls". The title string "Slider Controls" comes from a common code component called SliderControlsKeyboardHelpSection in scenery-phet, and there does not seem to be an option to change the value in the constructor. I have two questions about this for you:

  1. I'm going to assume that this is not blocking for the upcoming prototype release in Publish prototype version that includes the "Photons" and "Layer Model" screens #220. Agreed?
  2. Is this something we can simply do without, or should I create a common code issue and have someone make this an option for this dialog section? There would obviously be some complications to doing this, since providing a separate property to override the default might be confusing to anyone using phet-io to adjust strings. This isn't a deal breaker, but I thought it should feed into the decision.

@jbphet jbphet assigned arouinfar and unassigned jbphet and jessegreenberg Dec 20, 2022
@arouinfar
Copy link
Contributor

Thanks for investigating @jbphet.

  1. I'm going to assume that this is not blocking for the upcoming prototype release in Publish prototype version that includes the "Photons" and "Layer Model" screens #220. Agreed?

I completely agree. This is not important for the prototype.

  1. Is this something we can simply do without, or should I create a common code issue and have someone make this an option for this dialog section? There would obviously be some complications to doing this, since providing a separate property to override the default might be confusing to anyone using phet-io to adjust strings. This isn't a deal breaker, but I thought it should feed into the decision.

It seems like there should be an option in SliderControlsKeyboardHelpSection to provide a custom title. I'm surprised this doesn't already exist. There are sims that use slider interactions without having any visual sliders in the sim, such as Gravity Force Lab: Basics. In GFLB, there is a "Move Spheres" section in the keyboard help dialog, but perhaps this is a completely custom section and unrelated to SliderControlsKeyboardHelpSection.
image

I think the only PhET-iO implication is that the title string would live under model.strings.simName instead of model.strings.sceneryPhet. This seems like an acceptable trade-off to me.

@jbphet please open up a general issue.

@jbphet
Copy link
Contributor Author

jbphet commented Dec 22, 2022

@arouinfar - You were right, an option to change the section's title does exist, and I just missed it. I've made the changes, including some additional ones that you and I discussed over Slack. Please have a look and let me know if we're good to go.

@jbphet jbphet assigned arouinfar and unassigned jbphet Dec 22, 2022
@arouinfar
Copy link
Contributor

@jbphet the heading and contents look good, for Photons and Layer Model. However, there isn't a Flux Meter on the Waves screen, so it should just be the default SliderControlsKeyboardHelpSection.

@jbphet
Copy link
Contributor Author

jbphet commented Dec 23, 2022

However, there isn't a Flux Meter on the Waves screen, so it should just be the default SliderControlsKeyboardHelpSection.

Good point. I'd missed that, and have corrected it. The commits above should be cherry-picked to the release branch so that it is fixed in the prototype when deployed.

jbphet added a commit that referenced this issue Dec 23, 2022
jbphet added a commit that referenced this issue Dec 23, 2022
@jbphet
Copy link
Contributor Author

jbphet commented Dec 23, 2022

These changes have been propagated to the 1.1 branch. This should be verified before publication of the 1.1 prototype.

@Nancy-Salpepi
Copy link

This is what I see in rc.2:
-On the Photons and Layer Model Screens, the title is "Slider and Flux Meter Controls" (This change happened before rc.1)
-On the Photons and Layer Model Screens, "adjust slider" is just "adjust" (This change happened before rc.1)
-On the Waves Screen, the title has been changed back to the default "Slider Controls"
-On the Waves Screen, it says "adjust slider"
-The button correctly says "Option" when using a mac (yay!) instead of "alt"

I believe this list covers everything discussed in the issue, so closing.

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