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

Drop the "Property" suffix on the colorName argument to new ProfileColorProperty. #94

Closed
pixelzoom opened this issue Dec 15, 2021 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 15, 2021

For #84

  • All sims should use a color file named MyRepoColors.js or, if using abbreviations, MRColors.js, and
    use ProfileColorProperty where appropriate, even if they have a single (default) profile (to support color editing
    and PhET-iO Studio). The ColorProfile pattern was converted to *Colors.js files in PhET-iO instrumentation for ColorProfile scenery-phet#515. Please see GasPropertiesColors.js for a good example.

The first arument to new ProfileColorProperty is the colorName. The convention is to drop the "Property" suffix, as shown in the GasPropertiesColors.js example referenced in the checklist item.

In NumberPlayColors.js, the "Property" suffix is present, e.g. 'tenScreenBackgroundColorProperty' in:

  tenScreenBackgroundColorProperty: new ProfileColorProperty( numberPlay, 'tenScreenBackgroundColorProperty', {
    default: LIGHT_GREEN_BACKGROUND
  } ),

Compare to GasPropertiesColors.js, e.g. 'screenBackgroundColor':

  screenBackgroundColorProperty: new ProfileColorProperty( gasProperties, 'screenBackgroundColor', {
    default: 'black',
    projector: 'white'
  } ),
@Luisav1
Copy link
Contributor

Luisav1 commented Dec 15, 2021

Fixed in the commit above, back to @chrisklus for review and to close if everything is fixed.

@chrisklus
Copy link
Contributor

Looks great, thanks. 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