search

phetsims / arithmetic

"Arithmetic" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/arithmetic
GNU General Public License v3.0
5 stars 5 forks source link

Add dynamic locale #199

Closed marlitas closed 6 months ago

marlitas commented 7 months ago

THis issue will track work related to converting the sim to dynamic locale and it's usage of dynamic layout.

marlitas commented 7 months ago

Okay this issue is ready for review. Sending over to @Luisav1 or @AgustinVallejo. Whoever wants to get to it first.

One remaining TODO, that I need to check in with @pixelzoom about.

AgustinVallejo commented 6 months ago

Hi! Reviewed the commits and they look good to me. Only two questions @marlitas

  1. On the following commit, how come the margin changed so drastically? https://github.com/phetsims/arithmetic/commit/a9a4d23829da52e37e5af6809ff33fd07e9d819a

    -      yMargin: 38
+      bottomMargin: 180

  2. I've never seen this used, what is its purpose? Out of curiosity: https://github.com/phetsims/arithmetic/commit/9c3d6662e37a5987713c0dd9c6671c1299e15546

    +   ManualConstraint.create
marlitas commented 6 months ago

Thanks for the review @AgustinVallejo!

how come the margin changed so drastically?

I switched the AlignBox to align along the "bottom" of the layoutBounds since the buttons were jumping around as the font sized changed for the titles when the AlignBox was set to align along the "top". By switching to have alignBox track the bottom of its VBox contents I had to adjust the margin to go from the bottom of the layoutBounds to the bottom of the VBox content instead of measuring the space from the top of the layoutBounds to the top of the VBox content.

I've never seen this used, what is its purpose? Out of curiosity:

ManualConstraint is a way to specify a custom layout. I think of it a bit like a multilink for node bounds. Additionally ManualConstraint does some behind the scenes work to make sure the proxies that you are accessing in the callback are all in the same coordinate frame. I've been using it a lot in this dynamic layout work because it allows me to add in dynamic layout without doing major restructures to the sim's view architecture. It's probably a bit overload in a lot of the usage cases for the layout work I've been doing, but seemed better to go that route than to really tear up this older code that could potentially introduce other bugs and just isn't worth the time right now.

Let me know if there are any other questions or suggestions. Otherwise I think we can close!

AgustinVallejo commented 6 months ago

Thanks for the insightful answer! Closing :)

phet-dev commented 6 months ago

Reopening because there is a TODO marked for this issue.

marlitas commented 6 months ago

Oops connected TODO to a different issue.