phetsims / forces-and-motion-basics

"Forces and Motion: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/forces-and-motion-basics
GNU General Public License v3.0
7 stars 10 forks source link

Mass boxes misaligned relative to each other #252

Closed EthanWJohnson closed 6 years ago

EthanWJohnson commented 6 years ago

Not a big issue or bug, just a graphics/box alignment annoyance. If you look at the mass boxes in the second, third and fourth screens they are inconsistently sized/misaligned with each other image image image This is on a fresh load. Black bars for reference on where they align graphically. The differences are by only a few pixels but fairly noticeable; 1.) On the right boxes (the one with the man and girl) all boxes except for the man are 27 pixels in height. The man's box is 25 pixels tall, and this slightly squashes the lettering inside the box 2.) In the right box, both the present and trash can's mass box is aligned higher than the man and girl's box, by two and three pixels respectively. 3.) In the left box, the refrigerator mass box is 30 pixels tall, compared to the crate mass boxes which are 28 pixels tall. 4.) The refrigerator box is aligned one pixel higher than the crate boxes. 5.) On the fourth screen, the water bucket box is the only box aligned/sized consistently with another object, being aligned with the girl's box

I find the misalignment a bit odd since otherwise in the boxes each object is aligned with each other consistently on their lower sides. It's only a slight misalignment and not really a bug; just annoying to look at. It seems to be based on their full-scale sizes when pulled out as they are all roughly aligned/same sized as each other?

Found? in relation to https://github.com/phetsims/QA/issues/57, but it's technically ancient.

jessegreenberg commented 6 years ago

@ariel-phet should we spend time to clean this up? This is happening because the surrounding rectangles are sized by their value text, and the position of the mass labels is hard coded as

self.labelNode.bottom = normalImageNode.height - 2;
jessegreenberg commented 6 years ago

This is happening because the surrounding rectangles are sized by their value text

That doesn't explain why the standing man's label is smaller than the others. Im not sure why that is yet.

ariel-phet commented 6 years ago

@EthanWJohnson thanks for pointing this out

@jessegreenberg if it is not a huge amount of work to clean up, it would be a nice detail touch. This may be vestigial since this sim was so early on in HTML5 and we did not have some of our nicer alignment strategies developed yet.

I would say if more than 2 hours of work, mark deferred, but if fairly straightforward to clean up go ahead and take care of it.

jessegreenberg commented 6 years ago

Sounds good, I expect about an hour, but maybe less. I recall other alignment issues in the toolbox, here is at least one other that we did with S2015R: https://github.com/phetsims/forces-and-motion-basics/issues/161

For some reason that issue only considered acceleration screen.

jessegreenberg commented 6 years ago

For the sizing issue, i see this param in the constructor:

   // @param {number} homeScale - additional scale factor for when the item is in the toolbox

It is likely scaling down the whole node including the label.

jessegreenberg commented 6 years ago

@ariel-phet also suggested that we move the labels up a bit generally to give more space along the bottom.

jessegreenberg commented 6 years ago

Above commit ensures that the mass labels are always the same size. Still need to make sure that the items are aligned and add a bit more space.

jessegreenberg commented 6 years ago

Done in the above commits.

jessegreenberg commented 6 years ago

These commits need to be cherry-picked into release branches:

EthanWJohnson commented 6 years ago

Boxes are a lot better; can hardly tell the difference between them. Looks good!