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.

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

Add missing validation on slider #6982

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MarcSkovMadsen
Copy link
Collaborator

Closes #6963

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Jul 14, 2024

Complication: Editable Sliders have fixed start and end!

I've added the fix for #6963 including tests. But as you can see this makes a lot of other tests fail.

The reason is that its actually a feature of the editable sliders that you can set a value outside of the start and end!

image

I would need some guidance on what behavior we would like. I see different options:

  • Not wanting to validate values outside bounds.
  • Implementing different validations for non-editable and editable sliders
  • Changing the api of editable sliders such that values outside start and end are not supported. That would be a breaking change. But as I see the current API is problematic.

Complication: Order of inheritance

You will probably also have to revert the order of inheritance for editable sliders to be able to implement a custom validation in one place for them.

image

@MarcSkovMadsen MarcSkovMadsen changed the title Fix missing validation on slider Add missing validation on slider Jul 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can set FloatSlider values beyond start/end
1 participant