phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

VSlider has incorrect defaults for track size and knob dimensions #733

Closed samreid closed 2 years ago

samreid commented 2 years ago

Discovered in https://github.com/phetsims/bamboo/issues/50, if you create a default VSlider with default trackSize and thumbSize, the sliders look like this, but drag up and down:

image

So perhaps VSlider should use different defaults, like this (but with factored out constants):

    options = merge( {
      orientation: Orientation.VERTICAL,
      trackSize: new Dimension2( 5, 100 ),
      thumbSize: new Dimension2( 34, 17 )
    }, options );

Or perhaps if Slider detects orientation: Orientation.HORIZONTAL, it should swap widths and heights. Assigning @pixelzoom since this issue may be related to https://github.com/phetsims/sun/issues/380

pixelzoom commented 2 years ago

Looks like this problem was introduced in https://github.com/phetsims/sun/commit/53706aa5fef79d0805840dbd7e4a04b3a555ef9e for https://github.com/phetsims/sun/issues/733. I'll check it out.

pixelzoom commented 2 years ago

The problem is present in usages of VSlider that do not specify both options trackSize and thumbSize.

These uses specify both trackSize and thumbSize and do not exhibit this problem:

These uses do NOT specify both trackSize and thumbSize, and do exhibit this problem:

pixelzoom commented 2 years ago

Fixed in the above commits. Tested all sims listed in https://github.com/phetsims/sun/issues/733#issuecomment-1009379945, as well as a random sampling of HSliders.

Back to @samreid for review.

samreid commented 2 years ago

The changes seem good, thanks! Closing.