search

phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

Change GQSlider so it doesn't decorate with a label #208

Closed jessegreenberg closed 2 months ago

jessegreenberg commented 2 months ago

From https://github.com/phetsims/sun/issues/860, GQSlider was flagged as a component that decorates a slider with a label. Changed in the following commit, @pixelzoom can you please review or change how you would like?

pixelzoom commented 2 months ago

I'm not totally clear on why the Decorator pattern can't be used with VSlider. But assuming that's the case...

https://github.com/phetsims/graphing-quadratics/commit/f8f7f1800f2456532c9036675b92e454be71322d introduced a bug - when the slider is hidden in Studio, the label is not hidden. I fixed that in https://github.com/phetsims/graphing-quadratics/commit/3dc16663d60cfe46c51403ab69922f96952248b3.

I'm also not sure how I feel about passing VSliderOptions directly to the constructor of a Node subclass. It seems like the API should be changed. Let me ruminate on that a bit.

pixelzoom commented 2 months ago

I'm also not sure how I feel about passing VSliderOptions directly to the constructor of a Node subclass. ...

I addressed this in https://github.com/phetsims/graphing-quadratics/commit/143ad14693681863fb7c8ed3aaf5629a6769b625 by revising the options API for GQSlider.

Closing.