KeyboardHelpDialog goes out of dev bounds with stringTest=long

[jessegreenberg]

This was flagged in the RIAW RC in phetsims/resistance-in-a-wire#203 so I thought I would make an issue here before RC shas are grabbed. Assigning to @zepumph since he is working on this sim.

[jessegreenberg]

Also, phetsims/joist#546 was started to see if we can fix this generally, but nothing has come to mind yet.

[zepumph]

I agree that it would be nice to have a general solution. on hold until that is figured out.

[emily-phet]

@jessegreenberg Is the keyboard help dialog currently translatable? Regardless, if this is an easy RIAW-specific fix, let's do that and consider a general fix in the future.

[jessegreenberg]

The visible strings in the keyboard dialog are translatable but the a11y strings in the PDOM are not. I think we have a general fix in phetsims/joist#546 ready to go for Friction!

[zepumph]

This has been fixed in phetsims/joist#546. The dialog is looking very good with stringTest long. Closing

[jbphet]

Reopening. I just tested this on the latest Friction RC, the master version of Friction and the master version of Resistance in a Wire. It doesn't appear that the issue is fixed. Here's a screenshot from the Friction RC:

Here's one from RIAW. This one isn't as wide, but it's just a little past the dev bounds, which is what @KatieWoe originally reported in phetsims/resistance-in-a-wire#203.

This is not a problem for the current RC of RIAW since I have a workaround in place there, but should probably be addressed for the current Friction RC.

[jbphet]

@KatieWoe just came by and showed me that the reason I was seeing this is because I had a wide browser window. The keyboard dialog apparently scales with the size of the browser window. While this strikes me as a little odd, since other dialogs (such as the about dialog) don't do this, it does solve the problem of exceeding dev bounds in the cases where it matters. @zepumph - if this scaling behavior is known and has been approved by the powers that be, feel free to re-close.

[zepumph]

Interesting. I wonder if we need to tweak phetsims/joist@428b7a2 a bit more to get what we need.

[jessegreenberg]

I made phetsims/sun#435 to discuss during dev meeting.

[jessegreenberg]

In phetsims/sun#435 we decided that generally shrinking the Dialog is not something we can do, so we will need to set maxWidth on the help content labels within the dialog to ensure good widths for i18n.

[zepumph]

@jessegreenberg what strategy do you think is best in how we set up max widths. I feel like this issue is less of a "mark it work" kinda things, and more like a "let's get HelpContent.js and related files set up to make this as easy as possible for all future sims" kind of thing. I looked at lowering HelpContent.DEFAULT_TEXT_MAX_WIDTH and it wasn't obvious to me that this was going to fix much.

[zepumph]

Sounds good. Thanks for fine tuning this.

[jessegreenberg]

OK sounds good. I started to look into usages of HelpContent.DEFAULT_{{SOMETHING}}. One thing that would help with this is change labelWithIcon and labelWithIconList to take the label string rather than the the Text/RichText. Then clients don't even have to create the Text (nor do they have the ability to do so). Inspecting all usages, there are no cases where fonts other than the default are used.

[jessegreenberg]

OK, I think we are at a point where we should take remaining work to scenery-phet. The more work we do here, the more difficult it will be to bring into the release branch. And the rest of this doesn't impact friction (there might be slight differences in layout, but the original issue with the KeyboardHelpDialog has been resolved.)

[zepumph]

I feel good about that. My current plan is to cherry pick every commit here over to the branch. Let me know if you think there are any that could be omitted safely (though it doesn't really matter), or if there are some that specifically SHOULDN'T end up in Friction.

[jessegreenberg]

OK, sounds good. Master dialog in Friction looks like

Once we have commits in the rc branch, Ill continue to make changes in phetsims/scenery-phet#462.

[jessegreenberg]

And to clarify, I don't think there are any commits that need to be omitted.

[jessegreenberg]

Sorry, I went ahead with phetsims/scenery-phet#462. Let me know if I can help with the merging.

[zepumph]

Commit has been cherry-picked into branch (note to self)

[KatieWoe]

1.5.0-rc.2 this looks good