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

Slider min value #757

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Princic-1837592
Copy link

I'm working on a project where i need a slider to take values from 3 to 10, so i added a min_value field in the SliderView struct. This field is completely optional, defaults to 0 and doesn't break older versions of the library. The visual length of the slider is also adjusted according to this min value, as can be seen in the example

in example `slider.rs` a case with a non-zero min value is shown
@gyscos
Copy link
Owner

gyscos commented Oct 3, 2023

Hi, and thanks for the PR!

I think I omitted a "min value" from the slider to keep it minimal: the "max value" is always the length of the slider (no interpolation), and it's up to consumers to adapt this value to the desired range.

An alternative would be to let users set the valid range, and maybe even adapt to the available size: so a slider with 3 steps could be drawn, if needed, over a larger area, "skipping" steps, so the available modes could be:

[X----]
[--X--]
[----X]

This is a bit more complicated, since we must now map the "valid range" to the "display range", and not all cases are as simple as mapping 1-3 to 1-5.

I'm not entirely sure an intermediate solution is worth it: since the current number of steps is mostly display-based, the user will likely want to adapt the value to their valid range, and not just by adding the min value. So I fear that this is not enough for some users (who want to handle complex ranges), and not useful for others (who just want the "display-based" steps and adjust to their own ranges). That's just a subjective opinion, and I'm open to discussing it.

If we do want to add this, I have a few other nits: right now (I don't fully remember why), the max size is given at creation, and never changes. I would either keep that (taking the full range during creation), or change it entirely (letting users change both min and max value).

I'd also maybe store the valid range as a Range<usize>.

On a different note, maybe usize is also not the best type for steps here? u32 would probably be enough, and would be more architecture-independent. But then again that's a different problem that's present in the entire library, not just this type. 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants