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

Navbar: screen title maxWidth does not scale well with dynamic strings #884

Open
arouinfar opened this issue Nov 22, 2022 · 0 comments
Open
Assignees

Comments

@arouinfar
Copy link

Offshoot of phetsims/gravity-and-orbits#448

@arouinfar:

I find it odd that the first screen's maxWidth appears to be so much smaller than the second screen. Switching the localeProperty while the sim is running has a different appearance than loading the sim with the locale query parameter. Here's an example with locale=bs where the screen names have a similar character count.

Switching the localeProperty at runtime:
image

Initializing the locale with a query parameter:
image

It seems like the culprit may be the relative size of the English strings. "Model" is much shorter than "Real Molecules", so the first screen ends up with less width when dynamically changing the locale. Is this the expected behavior? It doesn't seem ideal to me, but I wouldn't consider it a publication-blocking issue, either.
image

Edit: screenshots were taken from the latest dev of Molecule Shapes (currently in QA testing)
https://phet-dev.colorado.edu/html/molecule-shapes/1.5.0-dev.4/phet/molecule-shapes_all_phet.html

@samreid replied:

It looks like in issue #438 we added this code:

const needsIconMaxWidth = options.maxButtonWidth && ( this.width > options.maxButtonWidth );
// Constrain text and icon width, if necessary
if ( needsIconMaxWidth ) {
text.maxWidth = icon.maxWidth = options.maxButtonWidth! - ( this.width - iconAndText.width );
}
else {
// Don't allow the text to grow larger than the icon if changed later on using PhET-iO, see #438
// Text is allowed to go beyond the bounds of the icon, hence we use `this.width` instead of `icon.width`
text.maxWidth = this.width;
}

This means the maxWidth of the screen icon is being set to its initial width. Should we just choose a constant for this, or perhaps a function that only depends on the number of screens?

Setting the maxWidth of a screen title to its initial width doesn't work well when supporting a dynamic locale, as illustrated in the example above. @samreid's suggestion to use a constant (or function based on the number of screens) sounds like a more robust option here. As I said previously, I don't see this as a blocking issue, just something that could be improved.

Assigning to joist responsible-dev @zepumph and dynamic layout expert @jonathanolson for their input and to evaluate the suggestion above.

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