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

Should "descriptions" be optional in APIs for keyboard-help content? #717

Closed
pixelzoom opened this issue Jan 4, 2022 · 7 comments
Closed

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 4, 2022

My understanding is that alternative input is becoming a standard part of PhET sims, while "descriptions" are not likely to be fleshed out for some time. So would it make more sense to make descriptions optional in the API?

Most (all?) of the API for creating keyboard-help content treats descriptions as required. They are required parameters to functions. Examples that I encountered while working on phetsims/geometric-optics#281 include:

  • KeyboardHelpSection.labelWithIcon
  • KeyboardHelpSection.labelWithIconList
  • KeyboardHelpSection.createJumpKeyRow

So I'm finding myself having to decide whether to pass the empty string '' to API calls, or create dummy descriptions in the strings.json file. The latter seems preferrable, but is an additional (and significant) cost.

@zepumph
Copy link
Member

zepumph commented Jan 4, 2022

Yes that is a correct summary. It should be an option. Our goal was to limit the confusion description. I'll see how many usages there are and try to take care of them, if its too much for today I may give up though.

zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/fourier-making-waves that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/gravity-force-lab that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/geometric-optics that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/faradays-law that referenced this issue Jan 4, 2022
@zepumph
Copy link
Member

zepumph commented Jan 4, 2022

labelWithIconList is complete. It took longer than I would like, but I kinda feel too in it now. I'll keep going.

@pixelzoom
Copy link
Contributor Author

I said:

So I'm finding myself having to decide whether to pass the empty string '' to API calls, or create dummy descriptions in the strings.json file. The latter seems preferrable, but is an additional (and significant) cost.

Now that descriptions will be optional, I'd prefer to remove those options from my sims that don't have descriptions (fourier, geometric-optics), and remove the placeholders from the strings.json file. Does that seem reasonable @zepumph?

@zepumph
Copy link
Member

zepumph commented Jan 4, 2022

Whatever you would prefer, but will you please wait for my refactor to complete?

@pixelzoom
Copy link
Contributor Author

Yes, I'll wait for this issue to be closed.

zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/fourier-making-waves that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/ratio-and-proportion that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/molecules-and-light that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/gravity-force-lab that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/geometric-optics that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/john-travoltage that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/coulombs-law that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/faradays-law that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/friction that referenced this issue Jan 4, 2022
zepumph added a commit to phetsims/joist that referenced this issue Jan 4, 2022
@zepumph
Copy link
Member

zepumph commented Jan 4, 2022

Alright. labelInnerContent is now an option. There were many helper functions that used the primary labelWithIcon function, so there were a fair number of functions changed in KeyboardHelpSection (along with their usages). local fuzzing is passing.

@pixelzoom, can you please review. Does this feel like a better API to you?

@pixelzoom, where you passed '', I just removed it, where you created a full dummy string, it is over to you.

@zepumph zepumph assigned pixelzoom and unassigned zepumph Jan 4, 2022
pixelzoom added a commit to phetsims/geometric-optics that referenced this issue Jan 4, 2022
pixelzoom added a commit to phetsims/fourier-making-waves that referenced this issue Jan 4, 2022
@pixelzoom
Copy link
Contributor Author

This is a much nicer API, and a better match for PhET's current "alt input now, descriptions later" approach.

In the above commits, I removed the description placeholders from geometric-optics and fourier-making-waves.

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

3 participants