phetsims / number-line-distance

"Number Line: Distance" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 3 forks source link

x1 and x2 labels overlapping tick mark labels in the explore screen #66

Closed amanda-phet closed 3 years ago

amanda-phet commented 3 years ago

@stemilymill wrote:

Another instance of x labels overlapping other things is in the explore screen, for the house/girl scene and thermometer scene, the x labels overlap the 0 on the number line.

x1 x2 labels overlap number line numbers zoom

Originally posted by @stemilymill in https://github.com/phetsims/number-line-distance/issues/63#issuecomment-892827027

amanda-phet commented 3 years ago

For these overlaps, I am wondering why the height of the x1 and x2 labels is so tall, especially above the variable. I think we can move these labels down if needed, but I would like to do a combination of first reducing the height of the label (if possible) and then shifting down if we still need more room to not overlap the 0.

For example, this pink rectangle looks like a totally reasonable size for the background of these labels: image

And then we can move them down a couple pixels so they don't overlap with the 0.

To be clear- this change can be applied universally: all screens and scenes.

amanda-phet commented 3 years ago

I might not be able to review this when it gets fixed, so if this change happens after August 7th please assign back to @stemilymill and also to @kathy-phet .

SaurabhTotey commented 3 years ago

I also found it odd how the backgrounds were a little too tall for the texts. I fixed this by forcing the texts to accurately compute their own bounds (as suggested by @jbphet). I also moved the point names down so that they no longer overlap with the number line numbers. Assigning to @stemilymill and @kathy-phet for review.

kathy-phet commented 3 years ago

Looks good to me! Thanks for the fix @SaurabhTotey.

samreid commented 3 years ago

Should this be cherry-picked into the RC branch?

kathy-phet commented 3 years ago

Yes, this should be in the 1.0 published version. I assume @jessegreenberg will help @SaurabhTotey with this process when rc.2 is published for spot checks.

jessegreenberg commented 3 years ago

@SaurabhTotey cherry-picked this change into the release branch, ready for verification by QA.