search

phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Upgrade joist screen icons/names and homescreen icons to use dynamic layout #828

Closed kathy-phet closed 2 years ago

kathy-phet commented 2 years ago

@arouinfar - Can you look at Studio and see if the icon layout is dynamically responsive if you hide one screen icon/name in the navigation and homescreens? If not, @jonathanolson, can you upgrade these layout groups to use dynamic layout?

jonathanolson commented 2 years ago

Implemented this part of the feature above, can you see how it looks?

arouinfar commented 2 years ago

@jonathanolson I think this is behaving as intended, but the home screen button looks a bit off in the absence of screen buttons. It seems like the screen button group (excluding the home button) is centered in the navbar and things re-center appropriately as individual screens are hidden. That all looks good, but I was surprised that the home button didn't end up centered when the last screen button was hidden.

That said, I don't think leaving behind only the home screen button is a realistic or desirable customization. I bring it up only in case this behavior is unintentional. If this is the expected behavior, no need to change anything.

image image image image image
jonathanolson commented 2 years ago

If this is the expected behavior, no need to change anything.

It's expected, since the layout is based on "the left-most visible screen button". To be backwards-compatible, I had to do some atypical things (the navbar screen buttons take up a certain amount of space for their layout, but the offset of the home button (a) doesn't respect this space/padding, it uses its own, and (b) only the screen button group is centered, that doesn't include the home button).

Thoughts on whether changing the appearance slightly could be beneficial?

arouinfar commented 2 years ago

Thanks for the explanation @jonathanolson. I'm fine with leaving things as-is.

Not sure if this issue can be closed or needs to be reassigned to another dev to review the changes, so back to @jonathanolson.

samreid commented 2 years ago

I tried hiding both navbar sim screen buttons in Gravity and Orbits and got this error:

assert.js:28 Uncaught Error: Assertion failed: x should be a finite number
    at window.assertions.assertFunction (assert.js:28:13)
    at HomeButton.translate (Node.ts:2341:17)
    at HomeButton.setRight (Node.ts:2951:12)
    at set right [as right] (Node.ts:2960:10)
    at set right [as right] (LayoutProxy.ts:180:5)
    at NavigationBar.ts:273:11
    at RelaxedManualConstraint.layout (RelaxedManualConstraint.ts:65:25)
    at RelaxedManualConstraint.updateLayout (LayoutConstraint.ts:189:14)
    at RelaxedManualConstraint.updateLayoutAutomatically (LayoutConstraint.ts:203:12)
    at TinyForwardingProperty.emit (TinyEmitter.ts:95:9)
samreid commented 2 years ago

Fixed in the commit. Everything else in this issue seems good to close. Please reopen if I've missed something or if there is more to do.