phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

standardize query parameter for "projector mode" feature #371

Closed pixelzoom closed 7 years ago

pixelzoom commented 7 years ago

Noted while working on https://github.com/phetsims/chipper/issues/516.

There are currently 2 different query parameters (projector and projectorMode) used to enabled the "projector mode" feature that is available in some sims.

Q1: Should PhET standardize this? Q2: Should it be in the QueryStringMachine schema for common code, so it's not repeated in sim-specific schemas?

Here are the clients of the 2 query parameters:

projector

projectorMode

pixelzoom commented 7 years ago

Since "projector mode" is a subset of the more general "color scheme" issue (see https://github.com/phetsims/joist/issues/222)... My vote is for colorScheme query parameter, defined like this in the common-code QueryStringMachine schema:

colorScheme: {
  type: 'string',
  defaultValue: 'default',
  validValues: [ 'default', 'projector' ]
}

Sample usage: ?colorScheme=projector

Molecule Shapes appears to have 3 color schemes: default, basics, projector (see MoleculeShapesColors). So we'll need to decide whether 'basics' is sufficiently common to add to the above schema, or whether Molecule Shapes should have a sim-specific schema for colorScheme.

jonathanolson commented 7 years ago

My vote is for colorScheme query parameter

Sounds good

So we'll need to decide whether 'basics' is sufficiently common to add to the above schema, or whether Molecule Shapes should have a sim-specific schema for colorScheme.

It seems sim-specific (or at least, we should have ways of having sim-specific color profiles).

samreid commented 7 years ago

It seems sim-specific (or at least, we should have ways of having sim-specific color profiles).

Agreed, there should be a sim-specific way of defining color profiles.

pixelzoom commented 7 years ago

Agreed, there should be a sim-specific way of defining color profiles.

There's nothing that limits a sim's ability to define a color profile. But there's currently no ability for a sim to modify the schema for a common-code query parameter -- i.e., no ability to add 'basics' to the validValues for colorScheme. Do we need to add this ability to QueryStringMachine? Or is it sufficient for a sim that needs such customization to copy-paste-modify the schema that it needs to extend?

pixelzoom commented 7 years ago

... i.e., Molecule Shapes would copy the colorScheme schema and modify it as follows:

colorScheme: {
  type: 'string',
  defaultValue: 'default',
  validValues: [ 'default', 'projector', 'basics' ]
}

... then use MoleculeShapesQueryParameters.colorScheme instead of phet.chipper.queryParameters.colorScheme. That could be problematic if there's common code that that handles general color schemes, and relies on phet.chipper.queryParameters.colorScheme.

pixelzoom commented 7 years ago

I asked:

But there's currently no ability for a sim to modify the schema for a common-code query parameter -- i.e., no ability to add 'basics' to the validValues for colorScheme. Do we need to add this ability to QueryStringMachine?

I don't see a way to do this. The schema is evaluated in preloads, way before the sim every exists. So there's no opportunity for a sim to (for example) add to validValues.

samreid commented 7 years ago

Regarding the proposal in https://github.com/phetsims/joist/issues/371#issuecomment-259763388

If you supply ?colorScheme=basics, then the validation in phetcommon would fail.

pixelzoom commented 7 years ago

Ah, yes. So the sim would also need to change the query parameter name, and that seems very undesirable.

pixelzoom commented 7 years ago

11/10/16 dev meeting notes: • we'll refer to these as "color profiles" • query parameter will be colorProfile • omit validValues from schema for colorProfile, to support sim-specific values

pixelzoom commented 7 years ago

The schema will be:

colorProfile: {
  type: 'string',
  defaultValue: 'default'
}

Sample use:

?colorProfile=projector

For sims that want to set a flag to enable projector mode:

var flag = ( phet.chipper.queryParameters.colorProfile === 'projector' )

I'll handle the changes.

pixelzoom commented 7 years ago

charges-and-fields was converted in https://github.com/phetsims/charges-and-fields/commit/78353b561fea847c6fc74d1e3a058b819aae4d51.

molecules-shapes was converted in https://github.com/phetsims/molecule-shapes/commit/1959d3a78715294e6909fd6948c4c66e308c53a8 and https://github.com/phetsims/molecule-shapes/commit/7baa6c8b16ff0c9212e51b11bb08e4d574731d90.

pixelzoom commented 7 years ago

All sims converted and tested. Projector mode is enabled with ?colorProfile=projector, default colors are used with anything else.

@jessegreenberg, you won the lottery - would you please verify?

jessegreenberg commented 7 years ago

I reviewed the commits above and tested all of the sims listed in https://github.com/phetsims/joist/issues/371#issue-188355688. I verified that the query parameter ?colorProfile works exactly as described in https://github.com/phetsims/joist/issues/371#issuecomment-260243749. A search through repos also verified that the list of sims in the initial comment included all sims that have a projector color profile.

@pixelzoom is that all for this issue?

pixelzoom commented 7 years ago

Yep, that's it. Thanks. Closing.