Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
jessegreenberg opened this issue Aug 19, 2024 · 2 comments
Closed

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

jessegreenberg opened this issue Aug 19, 2024 · 2 comments
Assignees
@pixelzoom
Labels
status:ready-for-review

Comments

@jessegreenberg
Copy link
Contributor

From phetsims/sun#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?
jessegreenberg added a commit that referenced this issue Aug 19, 2024
@jessegreenberg
Don't decorate UI components with content, see phetsims/sun#860 and #208
f8f7f18
pixelzoom added a commit that referenced this issue Aug 19, 2024
@pixelzoom
bug fix: hide label when slider is hidden,#208
3dc1666
@pixelzoom
Copy link
Contributor

pixelzoom commented Aug 19, 2024 •
edited
Loading

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

f8f7f18 introduced a bug - when the slider is hidden in Studio, the label is not hidden. I fixed that in 3dc1666.

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 added a commit that referenced this issue Aug 19, 2024
@pixelzoom
make GQSlider constructor protected, #208
98cd8ce
pixelzoom added a commit that referenced this issue Aug 20, 2024
@pixelzoom
revise the options API for GQSlider to reflect the new implementation…
143ad14 
… of Decorator pattern, #208
@pixelzoom
Copy link
Contributor

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

I addressed this in 143ad14 by revising the options API for GQSlider.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pixelzoom pixelzoom

Labels
status:ready-for-review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pixelzoom @jessegreenberg