bug changes the size of HSlider

[pixelzoom]

Discovered while working on Color Vision.

Here's how the "RBG Bulbs" screen is supposed to look (1.1.9 on the PhET website):

Here's how the "RBG Bulbs" screen looks in master. Note that all of the sliders are taller; the red slider is outside the layout bounds, and the blue slider is overlapping the Reset All button.

A bug was apparently introduced into HSlider for phetsims/scenery#603. If I roll back 0af28ff, then the problem goes away in Color Vision. And these lines are the culprit:

    // Dilate the local bounds horizontally so that it extends beyond where the thumb can reach.  This prevents layout
    // asymmetry when the slider thumb is off the edges of the track.  See https://github.com/phetsims/sun/issues/282
    this.track.localBounds = this.track.localBounds.dilatedX( thumb.width / 2 );

@samreid let's get this fixed. High priority since this could be affecting other sims.

[pixelzoom]

Perhaps this change to HSlider isn't properly accounting for cases like Color Vision, where an HSlider is rotated to a vertical orientation?

[pixelzoom]

Spot checking HSlider in several other sims, I don't see any problem. So I'm more convinced that the offending code is broken for vertical sliders.

[samreid]

The HSlider change made the slider 8px wider. RGBSlider was adding 30px for its margin. If we subtract off 8 from that margin, everything looks nice again (and we can retain the HSlider fix where the slider bounds don't change when the thumb goes off the edge of the track):

  function RGBSlider( intensityProperty, color, tandem ) {

    var hSlider = new HSlider( intensityProperty, { min: 0, max: 100 }, {
      trackSize: new Dimension2( 100, 2 ),
      thumbSize: new Dimension2( 14, 28 ),
      thumbTouchAreaXDilation: 7,
      thumbTouchAreaYDilation: 7,
      tandem: tandem
    } );
    hSlider.rotation = -Math.PI / 2;

    var rectWidth = hSlider.width + 8;
    var rectHeight = hSlider.height + 30 - 8; // I subtracted off 8 in this line

Hence my understanding of the problem is that the slider track and hence the slider got wider in 0af28ff and some sims such as color vision would need to have their layout updated to deal with the wider slider.

Reassigned to @pixelzoom for discussion.

[pixelzoom]

That indeed fixes the issue for Color Vision. But what about other sims that are doing computations based on the slider width (or height, if the slider is rotated)? Assuming that all HSlider instances are assigned to a var named "*Slider".... There are 9 occurrences of "lider.width", and 2 occurrences of "lider.height".

[samreid]

@ariel-phet, a recent change to HSlider fixed a layout problem but also increased its width by 8 pixels. @pixelzoom pointed out how this led to a layout problem for color vision "rgb bulb" screen, see above. @ariel-phet Should we look through the rest of our sims and see if there are slider-related layout problems? Can you please prioritize and delegate?

[ariel-phet]

@samreid I imagine this will just require a quick visual inspection of all sims on master? Correct?

[samreid]

@pixelzoom do you think a quick visual inspection of all sims on master in one browser would be sufficient?

UPDATE: I suspect a quick visual inspection of all sims on master in one browser would be sufficient, but I wanted to double check with @pixelzoom

[pixelzoom]

Visual inspection on one browser should be sufficient to discover any serious problems (like Color Vision). Comparison with published version would be needed to discover any less obvious layout differences, then decide whether the differences are important.

[samreid]

Thanks for your comment @pixelzoom, back to you @ariel-phet.

[ariel-phet]

I will take a look

[ariel-phet]

Sims with sliders

• Acid-Base Solutions
• Atomic Interactions
• Balancing Act
• Beer's Law Lab
• Bending Light
• Color Vision
• Concentration
• Energy Skate Park: Basics
• Forces and Motion: Basics
• Gravity And Orbits
• Gravity Force Lab
• Hooke's Law
• Isotopes and Atomic Mass
• Least-Squares Regression
• Molarity
• Molecules and Light
• Neuron
• ❌ Ohm's Law
• pH Scale
• Plinko Probability
• ❌ Resistance in a Wire
• Rutherford Scattering
• States of Matter
• States of Matter: Basics
• Under Pressure
• Wave on a String

[ariel-phet]

Ohm's law on master:

Ohm's law published:

[ariel-phet]

I compared published and master on Windows Chrome for all sims with sliders.

@samreid looks like Ohm's law and RIAW are not passing, but all other sims look good.

Can you or @zepumph fix up RIAW and Ohm's Law?

[zepumph]

@samreid looks like Ohm's law and RIAW are not passing

@ariel-phet I would love a little bit of explanation on what there is to be done for this issue. I think that the main reason that you are seeing a difference between master on the phet site with these two, is because they were originally published with a custom slider implementation, and have since been moved over to master using HSlider. Here is RIAW

In master

And on the PhET Site:

It seems like there are definitely different margins, and the track is a little bit longer in master (not the actual track, but the total range of the slider). As a result the slider is pretty close to the the letters above the slider. Is this what you would like me to change, by decreasing the 'height' of the slider by 10 or so pixels so that it has a better margin?

[ariel-phet]

@zepumph - yes basically the length of the black track should match the published version in both Ohm's Law and RIAW (currently on master the black track is quite short vertically)

[samreid]

@ariel-phet HSlider does not currently support having the edge of the knob goes to the edge of the track. It only supports having the middle of the knob going to the edge of the track.

This changed when @veillette rewrote the sim to use HSlider instead of a custom/independent implementation.

[ariel-phet]

@samreid @zepumph please replace the "custom" slider thumb in RIAW and Ohm's law with the standard slider thumb (but with a "silverish" coloring). Then extend the track more to match the original.

[pixelzoom]

The space between the bottom of the track and the label below it is probably also relevant. There is excessive space in the screenshots taken from master.

[zepumph]

I just need to do RIAW now.

[zepumph]

RIAW is done now. I will get feedback during the dev test. See above commit.

[zepumph]

@ariel-phet supplied designer feedback for the color. Closing