fix alignment of equation terms above sliders

phetsims / curve-fitting

"Curve Fitting" is an educational simulation in HTML5, by PhET Interactive Simulations.

GNU General Public License v3.0

6 stars 3 forks source link

fix alignment of equation terms above sliders #80

Closed veillette closed 5 years ago

veillette commented 8 years ago

The process of making the strings translatable broke the alignment of the equation with the slider

pixelzoom commented 8 years ago

This is presumably referring to the sliders that appear when "Adjustable fix" is selected, and the terms that each of those sliders is associated with.

pixelzoom commented 7 years ago

Assigning to @memo330179, since this is one of the "3 specific issues" to work on first.

memo330179 commented 7 years ago

I need some clarification on this issue. I am not sure what the alignment issue is. My first instinct says the sliders all should be in the center, but I am not sure.

pixelzoom commented 7 years ago

The sliders should be positioned below their respective terms in the equation. Here's a mockup from the design document. Note how the slider for 'a' is centered below the 'a' in the equation (and similarly for other terms).

screenshot_63

SaurabhTotey commented 5 years ago

I have made a few small commits to try and reduce this issue. The sliders are right now slightly better aligned under the coefficients, but it still isn't quite perfect. I have spent a lot of time trying to get perfect alignment (using things like .centerX) to no avail, so I have instead used an HStrut and a modification of slider dimensions as a stopgap. Below are a few pictures of how it looks as of this current moment.

pixelzoom commented 5 years ago

Trying to eyeball the slider locations is problematic, since font sizes vary on browsers, and translated symbols will change term width. If you want to precisely position the sliders under terms, you might try adding methods to the equation that allow you to get the position of each term. That will give you the information needed to position the sliders. You may need to transform the term position (using Node's coordinate transform methods, e.g. , parentToGlobalPoint, globalToParentPoint) to put it in the slider's coordinate frame. Let me know if I can help.

SaurabhTotey commented 5 years ago

Thanks for the offer for help! Eventually, I was able to figure out why my previous attempts at setting centerX didn't work. After a few hours of trying to figure out why my solutions weren't working, I figured out that I was having issues with HBox and VBox ignoring the fact that I was setting a new centerX. I just fixed this by replacing the usages of HBox and VBox with Node and spacing elements relative to each other. Now the sliders are exactly under coefficients. I have included images below.

It seems like the c and d sliders are closer to each other than other sliders because the linear term of the equation doesn't have an exponent to provide it extra spacing. I am wondering if this correct/desired behaviour, or whether the equation should have different spacing. Regardless, the sliders now align exactly under the coefficients.

pixelzoom commented 5 years ago

@SaurabhTotey wrote:

... Now the sliders are exactly under coefficients.

Nicely done!

... I am wondering if this correct/desired behaviour, or whether the equation should have different spacing.

In Graphing Quadratics, the difference in horizontal space between sliders was OK, see screenshot below. But you should check with @amanda-phet, since this sim may have requirements that I'm not aware of.

SaurabhTotey commented 5 years ago

Assigning to @amanda-phet to review and either give feedback or sign off. The feedback on this issue will potentially affect how I will need to attempt #72.

amanda-phet commented 5 years ago

Since these sliders are much closer than the ones in GQ, I have a bit more hesitation. c and d are just really close and it seems a problematic from a touch perspective and a little unnecessary. I would prefer if they were more evenly spaced, even if they aren't directly under the coefficients (sorry!). The spacing between a, b, and c looks good, so maybe just apply that to all of the sliders.

SaurabhTotey commented 5 years ago

Now in master, the sliders are placed relative to each other and the left edge and have an even spacing. This comes at the expense that the sliders aren't guaranteed to be exactly under the coefficients, which I don't think should be a huge issue. This is what it looks like now.

amanda-phet commented 5 years ago

Checked on master and they look great to me! Thanks for making that change.