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

Update A11yStrings conventions and practices to make it easier to convert to i18n later #65

Closed
zepumph opened this issue Nov 28, 2017 · 13 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Nov 28, 2017

We know that we will be offering a11y strings to translators at some point, we just don't know when. The more that we can do now to sync up our practices of a11y strings to the way we use normal strings, the easier that conversion will be. Right now a11y strings are not in the same format as their json counterpart:

a11y:

  var OhmsLawA11yStrings = {
    resistanceUnitsPatternString: '{{value}} Ohms',
    voltageUnitsPatternString: '{{value}} Volts',
    resistanceSliderLabelString: 'R, Resistance',
...
}

normal:

{
  "acid": {
    "value": "Acid"
  },
  "acid-base-solutions.title": {
    "value": "Acid-Base Solutions"
  },
  "base": {
    "value": "Base"
  },

I think that there are a few things that we can do to make it easier on us later.

  1. Make sure that all a11y strings for a file are declared as vars at the top of the page. This is already being done for the most part, but every now and then we find one inline. Since i18n strings will need to be required, a block at the top, with variable declaration, will bridge the transition quite well. Example of bad practice (we've all done it): accessibleLabel: BASEA11yStrings.wallLabelString,

  2. Convert from an object schema {{key}}: {{string}} to {{key}}: { "value": {{string}} }. This one would require a fair bit of work, going through each a11y string so far, but we will need to do that eventually, so I say lets bite the bullet now. This will require visiting all the call sites and adding ".value" to the end of the variable declarations. Kinda sloppy, and I would like @jessegreenberg's opinion before proceeding.

Then when it is time to do the transfer, all we will need to do is:

  1. change variable declarations to use require and the string! plugin (removing the ".value" from the end of the line.
  2. Move all of the a11y strings into the normal strings json files. This should just be copying verbatim code.

I'm happy to do this grunt work now, especially since it will save so much time later. I'll just wait for @jessegreenberg to comment/give the go ahead. Especially since it would involve touching sims that he is actively working on like BASE.

@zepumph
Copy link
Member Author

zepumph commented Nov 28, 2017

@jessegreenberg messaged me on slack with this question:

Convert from an object schema {{key}}: {{string}} to {{key}}: { "value": {{string}} }

I am still not quite sure how this would work.
So at declarations you would add .value to end like
var greenBalloonLabelString = BASEA11yStrings.greenBalloonLabelString.value
Then change the .js a11y string files to .json files?

I say:

You are close. Your declaration line aligns with what I was thinking. I do not recommend changing to .json files though. Instead the .js file would look like:

  var OhmsLawA11yStrings = {
    resistanceUnitsPatternString: {
      value: '{{value}} Ohms'
    },
    voltageUnitsPatternString: {
      value: '{{value}} Volts'
    },
    resistanceSliderLabelString: {
      value: 'R, Resistance'
    },

@jessegreenberg
Copy link
Contributor

Thanks @zepumph, this sounds good. +1 for leaving as .js files. I agree that this will reduce our work load later on. Removing my assignment, but if you would like to split up the work please reassign to me.

@jessegreenberg jessegreenberg removed their assignment Nov 28, 2017
zepumph added a commit to phetsims/molecules-and-light that referenced this issue Nov 29, 2017
zepumph added a commit to phetsims/scenery-phet that referenced this issue Nov 29, 2017
@zepumph
Copy link
Member Author

zepumph commented Nov 29, 2017

I'm going to convert scenery-phet strings now because I broke xss fuzzing yesterday when I added a few new object keys into that file.

zepumph added a commit to phetsims/molecules-and-light that referenced this issue Nov 30, 2017
@zepumph
Copy link
Member Author

zepumph commented Dec 8, 2017

Another things we can do to help ourselves is name keys in the string files without the "String" suffix, but include those suffixes in the variable names in the js files. This is enforced with the string! plugin by eslint. Tagging @jessegreenberg so he is aware.

image

@jessegreenberg
Copy link
Contributor

Roger roger.

zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/reactants-products-and-leftovers that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/gravity-force-lab-basics that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/rutherford-scattering that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/resistance-in-a-wire that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/molecules-and-light that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/plinko-probability that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/john-travoltage that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/build-an-atom that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/coulombs-law that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/molarity that referenced this issue Mar 6, 2018
zepumph added a commit to phetsims/ohms-law that referenced this issue Mar 6, 2018
@jessegreenberg
Copy link
Contributor

jessegreenberg commented Mar 7, 2018

@zepumph did a search and found that these are the repos that need to be updated.

- [ ] john-travoltage [@mbarlow12]
- [ ] joist [@zepumph]
- [ ] ohms-law [@zepumph]
- [ ] gravity-force-lab-basics [@mbarlow12]
- [ ] balloons-and-static-electricity [@jessegreenberg]

We will work on this together, dev names have been added to the repos that we will each work on.

EDIT: We decided to create separate issues for this.

@jessegreenberg
Copy link
Contributor

We have created new issues in each repo for this. Closing this issue.

@jessegreenberg
Copy link
Contributor

Reopening to make sure that all a11y strings are loaded at the top of the files (like the string plugin). @zepumph volunteered to do this.

@zepumph
Copy link
Member Author

zepumph commented Mar 7, 2018

The repos mentioned above (with separate issues) will cover those sims, but I will look at:

  • Friction
  • Scenery Phet
  • Molecules and Light
  • RIAW

in this issue because they all already have the right structure for their string files.

@mbarlow12
Copy link

mbarlow12 commented Mar 13, 2018

Working through phetsims/gravity-force-lab-basics#63, and I just wanted to confirm that we're NOT going the json and require route, correct?

@zepumph
Copy link
Member Author

zepumph commented Mar 13, 2018

Correct, we are just setting ourselves up to make that easiest in the future. Thanks for checking.

zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue Mar 15, 2018
zepumph added a commit to phetsims/resistance-in-a-wire that referenced this issue Mar 15, 2018
zepumph added a commit to phetsims/john-travoltage that referenced this issue Mar 15, 2018
zepumph added a commit to phetsims/ohms-law that referenced this issue Mar 15, 2018
@zepumph zepumph closed this as completed Mar 15, 2018
zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue Mar 15, 2018
zepumph added a commit to phetsims/scenery-phet that referenced this issue Mar 15, 2018
jessegreenberg pushed a commit to phetsims/greenhouse-effect that referenced this issue Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants