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

level of phetioDocumentation? #61

Closed
pixelzoom opened this issue Oct 17, 2018 · 4 comments
Closed

level of phetioDocumentation? #61

pixelzoom opened this issue Oct 17, 2018 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Oct 17, 2018

This issue will help me decide how to address #55.

In Graphing Quadratics, I have coordinatesVisibleProperty, which is specific to the view. It controls the visibility of coordinates that appear on a screen. How specific should phetioDocumentation be for this Property? (vertexProperty has a similar issue, but let's stick with discussion of coordinatesVisibleProperty for now.)

Which of these 4 options is preferred?

(1) Screen-specific phetioDocumentation, i.e.:

Standard Form screen: 'whether coordinates are visible on the vertex and roots'
Vertex Form screen: 'whether coordinates are visible on the vertex'
Focus & Directrix screen: 'whether coordinates are visible on manipulators (vertex, focus, point on parabola)'

I'm not convinced that this level of specificity is even useful.

(2) General phetioDocumentation used for all screens, i.e.:

'whether coordinates are visible'

For coordinatesVisibleProperty, this seems highly redundant, similar to documenting getWidth as "Gets the width".

(3) No phetioDocumentation. I'm leaning heavily towards this option.

(4) More verbose screen-specific Property names, possibly combined with (1), e.g.:

vertexAndRootsCoordinatesVisibleProperty
vertexCoordinatesVisibleProperty
manipulatorCoordinatesVisibleProperty

This option hides the fact that this Property has the same general purpose on all screens. It also limits possibilities for reuse of the Property across screens.

@zepumph
Copy link
Member

zepumph commented Oct 17, 2018

I think that number one seems like the most correct, and taking a short look into VertexFormViewProperties it seems like it would be possible to use nested options to pass the more specific options to each of the instances of the viewProperties.

I would also be ok with a more general doc that covers ever use case. For coordinatesVisibleProperty that would look perhaps like "whether coordinates are visible on plotted points on the graph"

What do you think of those two options?

@pixelzoom
Copy link
Contributor Author

Thanks for the feedback @zepumph.

I don't like the complication/indirection of passing phetioDocumentation via options. I prefer to have phetioDocumentation co-located with the Property definition.

I could live with (2) with 'whether coordinates are visible on plotted points and manipulators on the graph'. Throughout design and implementation, we've used "plotted point" to refer to a non-interactive point that is plotted on the graph. Manipulators are interactive points used to modify a curve. Some screens have plotted points, some screens have manipulators, no screens have both, one screen has neither.

@zepumph
Copy link
Member

zepumph commented Oct 17, 2018

plotted points and manipulators

  1. maybe use "plotted points and/or manipulators"
  2. although there is a terminology built up internally. I don't necessarily think it will always be the best way to communicate to the public client. Maybe something simple and general, that won't confuse devs could be nicer: "displayed points" or something.

@zepumph zepumph removed their assignment Oct 17, 2018
pixelzoom added a commit that referenced this issue Oct 17, 2018
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 17, 2018

I agree with 2 above. I'm going with:

// @public
this.coordinatesVisibleProperty = new BooleanProperty( GQQueryParameters.checkAll, {
  tandem: options.tandem.createTandem( 'coordinatesVisibleProperty' ),
  phetioDocumentation: 'whether (x,y) coordinates are visible on points that are displayed on the graph'
} );

pixelzoom added a commit that referenced this issue Oct 17, 2018
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