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

Limit width of content in KeyboardHelpDialog #546

Closed
jessegreenberg opened this issue Dec 19, 2018 · 8 comments
Closed

Limit width of content in KeyboardHelpDialog #546

jessegreenberg opened this issue Dec 19, 2018 · 8 comments

Comments

@jessegreenberg
Copy link
Contributor

KeyboardHelpDialog currently assumes that its content will keep the width within the dev bounds. But this means that we generally need to define a max width on the content every time we create a KeyboardHelpDialog. In phetsims/resistance-in-a-wire#203 @jbphet recommended

it seems like it would be better to do this generally so that KeyboardHelpDialog is automatically limited to the dev bounds.

Assigning to myself to investigate.

@jessegreenberg jessegreenberg self-assigned this Dec 19, 2018
@jessegreenberg
Copy link
Contributor Author

This issue is not specific to KeyboardHelpDialog but all Dialogs. Dialogs don't have knowledge of the dev bounds and are often not attached to a ScreenView so it doesn't seem like a Dialog can do this generally.

@jessegreenberg
Copy link
Contributor Author

What if we have a supertype KeyboardHelpDialogContent that has a default maxWidth that works well for default layout bounds of ScreenView but can be specified as an option for sims that are not using default layout bounds?

@zepumph
Copy link
Member

zepumph commented Dec 19, 2018

I found phet.sim.joist.screenBoundsProperty (joist-internal) (read-only). What if we just used a percent of that?

@zepumph zepumph self-assigned this Dec 19, 2018
zepumph added a commit to phetsims/resistance-in-a-wire that referenced this issue Dec 19, 2018
@zepumph
Copy link
Member

zepumph commented Dec 19, 2018

@jessegreenberg please review. Do you think that this kind of solution could work?

@jessegreenberg
Copy link
Contributor Author

Thanks @zepumph, something like this could work. Dialog has a @private reference to sim so maybe we can make that @protected and use in KeyboardHelpDialog.

Only a single instance of KeyboardHelpDialog is created so the maxWidth will change depending on whether the dialog is opened on the home screen vs a sim screen. Instead of setting on construction maybe we can update the maxWidth on show.

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented Dec 19, 2018

Ah, I found that Dialog has a layoutStrategy option that has everything that we need!

jessegreenberg added a commit that referenced this issue Dec 19, 2018
@jessegreenberg
Copy link
Contributor Author

I tested this in RIAW, friction, and coulombs-law. @zepumph can you please review?

@zepumph
Copy link
Member

zepumph commented Dec 19, 2018

This looks awesome! Well done. I will note in friction, and let you handle RIAW.

@zepumph zepumph closed this as completed Dec 19, 2018
jbphet pushed a commit that referenced this issue Dec 27, 2018
…he simulation bounds, see #546

(cherry picked from commit 428b7a2)

# Conflicts:
#	js/KeyboardHelpDialog.js
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

2 participants