search

rio-labs / rio

WebApps in pure Python. No JavaScript, HTML and CSS needed
https://rio.dev
Apache License 2.0
453 stars 14 forks source link

Added show_values to the slider component #47

Closed JJFlorian closed 2 weeks ago

JJFlorian commented 3 weeks ago

Hey devs! Another PR by me. I missed this option. Hope I didn't break something I am not aware off. For me it worked fine!

setValueFromMouseEvent was changed to make sure the current value is updated while sliding, and not only after dropping the knob on the new position.

Styling isn't my strength, so of course feel free to change it.

What does it do?

I added an option to show the min, max and current value of the Slider component. They can be toggled on or of with the show_values parameter, which is True by default

Why is it needed?

Just an upgrade of the Slider. Makes it visually easier to work with as user

How to test it?

Just add a Slider

Related issue(s)/PR(s)

None

JJFlorian commented 3 weeks ago

The component now reacts to changes to the state during its lifetime.

It is done by blocking the display in the show_value related elements, so those elements are always present.

In other components I saw the html dynamically being added, but that made it more complex imo. This works fine, and I dont see any issues. Let me know if you guys prefer the other method

Aran-Fey commented 3 weeks ago

Looks good. Just FYI, it'll probably be a few days before we merge this because we're currently caught up in other things. Once the more pressing stuff is taken care of, we'll quickly discuss our design goals for sliders among the team and then maybe make some minor changes to the code.

JJFlorian commented 3 weeks ago

Oh yea sure. No worries

mad-moo commented 2 weeks ago

I've merged this, though with a caveat. Ideally, show_values could be extended even further - see #63

Rio is currently undergoing a layout rework, that might necessitate rewriting a lot of components. Because of that I'd hold off of further impreovements for this right now. Keep it as is, and once layouting is done we can implement #63.

I've also removed the currently selected value. I think that one could actually always be displayed, but only while the slider is hovered. Maybe something like this image

Or here's the M3 equivalent: image

Because of that, I don't think it's actually related to displaying the values. This is also something we can add after the rework.

Thanks @JJFlorian !