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

Feature/inverted slider #4705

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

Conversation

ccric
Copy link

@ccric ccric commented Feb 28, 2024

Adds a new inverted property to the slider widget to change the direction of the slider.
PR includes needed adaptations for fluent, cosmic, material and cupertino styles.
Tested with different values and horizontal as well as vertical orientation.

@CLAassistant
Copy link

CLAassistant commented Feb 28, 2024

CLA assistant check
All committers have signed the CLA.

@tronical
Copy link
Member

Thanks for the PR :). Could you explain to us what the use-case is for this?

@ccric
Copy link
Author

ccric commented Feb 29, 2024

Thanks for the PR :). Could you explain to us what the use-case is for this?

I'd love to!
As a developer developing a UI for a component where values shall be adjustable by the user, as well as set by an external API, I think I need to use in-out properties with two-way bindings.

For the user using the UI, a vertical slider with the highest value on top is desired, because that feels more intuitive than having the high value at the bottom, as it is right now.

One solution to this could be to give the two-way binding some bijective function to invert the values, but I could not figure out how to do this.
Using two one-way bindings with expressions did not work either.

So I figured that one easy to use approach would be to invert the slider. If this behavior could be achieved another way, I would be happy if you could explain it to me.

I could certainly implement a custom widget with this behavior, but I think this use case is common enough for a general implementation.

@tronical
Copy link
Member

tronical commented Mar 4, 2024

Ahhh, now I see. Thank you!

Perhaps we've made a mistake in our API design here, and we should have from and to instead of minimum and maximum.

We could rename those and keep the old names for compatibility.

Such a direction would be my preference over a boolean inversion.

Florian, what do you think?

@ccric
Copy link
Author

ccric commented Mar 4, 2024

That would be cleaner, yes.
I was not sure about how you handle API breaks, so I tried a solution without breaking existing apps, first.

@ogoffart
Copy link
Member

ogoffart commented Mar 4, 2024

We should also look at the guidelines from the platform and depending we might need to make sure that it is in the right direction by default

@tronical
Copy link
Member

tronical commented Mar 4, 2024

We should also look at the guidelines from the platform and depending we might need to make sure that it is in the right direction by default

Could you clarify what platforms you have in mind? Or instead of platforms, do you mean right-to-left vs. left-to-right when used in locales where we want to mirror layouts?

@tronical
Copy link
Member

tronical commented Mar 4, 2024

On further thoughts, in my opinion, in the short term this particular feature - support for inverted ranges - could be implemented without changing the API but by automatically supporting the case when the minimum is greater than the maximum and implicitly "inverting".

Basically treating "minimum" as "from" and "maximum" as "to" - as if we had given them these names from the beginning.

@ogoffart
Copy link
Member

ogoffart commented Mar 5, 2024

Could you clarify what platforms you have in mind?

I mean Windows for fluent, Mac for cuppertino and so on.

For example, the fluent design says:

https://learn.microsoft.com/en-us/windows/apps/design/controls/slider#range-direction
For vertical slider, put the largest value at the top of the slider, regardless of reading direction. For example, for a volume slider, always put the maximum volume setting at the top of the slider.

But we have it by default in the wrong direction:
This is our fluent vertical slider: As you can see, this is upside down (the blue part should be on the bottom)
image

Not sure what mac does.

But I'd say the first thing we should do before having this feature is to fix the vertical slider so it's not upside down by default.

@tronical
Copy link
Member

So there are two issues:

  1. The implementation of the Fluent style seems wrong.
  2. We should make it possible to implement an inverted slide by allowing people to swap minimum and maximum. This is an edge-case that we can document but we don't necessarily need to rename minimum/maximum to to/from.

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.

5 participants