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

projectorModeCheckbox PhET-iO tree structure #654

Closed
zepumph opened this issue Aug 20, 2020 · 4 comments
Closed

projectorModeCheckbox PhET-iO tree structure #654

zepumph opened this issue Aug 20, 2020 · 4 comments

Comments

@zepumph
Copy link
Member

zepumph commented Aug 20, 2020

Over in phetsims/states-of-matter#346, @jbphet and I realized it was weird that there was a linked property right next to the checkbox. I think it would be best to not link there, and instead just have the passed in property be called "property" I will take a look.

Here is what the structure looks like:

image

@zepumph
Copy link
Member Author

zepumph commented Aug 20, 2020

This is now the new structure. Notice that it isn't blue, because it isn't linked, but we also don't have an extra projectorModeEnabledProperty next to the linked property:

image

@samreid please review the commits.

@arouinfar please review if this is an alright cleanup change for the studio tree structure, and let me know if you think this should be added to SOM. if you do think it should be added. This can not and should not be included in a later MR, since it breaks the API, thus it would have to be added for the release tomorrow, or never added. Marking high priority in case you want to squeeze it in (I doubt that is what we want to do).

@arouinfar
Copy link

@zepumph the tree structure looks good to me.

@kathy-phet @jbphet and I discussed this a bit in phetsims/states-of-matter#346, and we decided it wasn't critical for the 1.2.0 release. Since it can't be included in a MR, I'm fine with leaving it out. Clients should be using the query parameter for projector mode, anyway.

@samreid
Copy link
Member

samreid commented Aug 21, 2020

This looks like the optimal implementation to address these issues, nice work! I'm also glad we tried out the strategy to make API calls to linked elements work the same as calls to core elements. I don't have any recommendations for the review. I don't mind that it won't be in States of Matter 1.2.0, it will go out with other PhET-iO simulations published after today. @zepumph can this be closed?

@zepumph
Copy link
Member Author

zepumph commented Aug 26, 2020

Clients should be using the query parameter for projector mode, anyway.

I see that colorProfile is used in client guides but isn't marked as "public" I also see that for ?locale. I want to add that to those. @arouinfar please note that any query parameter discussed in those guides should be marked as "public:true" in the schema (in initialize globals). This adds support for the graceful dialog. It can be really hard to keep track of all the public usages, so another set of eyes would be nice to have. See phetsims/chipper#974 to fix that.

Otherwise I am ready to close.

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