feat: Improve ticks on Sliders

[Lundez]

Before	After

I'm not sure about thumb_size change.

This change improves the UX and makes apps clearer in selections.

Changes:

1. Set the thumb_size=24 to make it not as large.
2. Introduce show_end_ticks which is defaulted into True and shows the min/max ticks.
3. Improve typing of thumb_label and default into always to make selection clear.

[Lundez]

Failure is partly because older versions of python don't support Literal. How do you prefer I handle this @maartenbreddels ?

[maartenbreddels]

Thank you for the PR btw :)

[Lundez]

I'm not sure about the show_end_ticks. Should we just expose tick_labels so a user can set it?

Regarding "show_end_points" how about giving multiple option and defaulting to showing end-points only?

I think min&max makes a lot of sense for numeral ranges. It makes the UI clearer and easier to work with. A screenshot of the app will also tell the full story. That's my opinion, but I'll do what you prefer as you're the maintainer 😊👍

[maartenbreddels]

I'm really torn on this PR. On the one hand, I really understand the need and see the benefit of this PR, but I think it interferes with the generality of the vuetify slider. Especially if we implement https://github.com/widgetti/solara/issues/203 it's unclear what should happen when a user passes tick_labels, and also enables this option.

[Lundez]

I'm really torn on this PR. On the one hand, I really understand the need and see the benefit of this PR, but I think it interferes with the generality of the vuetify slider. Especially if we implement #203 it's unclear what should happen when a user passes tick_labels, and also enables this option.

I see, this Issue did not exist before this one I believe. We could redefine tick_labels to : list | Literal['end_points'] | None and customize that behaviour.

The behaviour of showing end-points & "thumb-label" makes a lot of sense if Solara should have good defaults, IMO. And adding "end-points" manually would be painful as a user (as you can see on my code). It does make sense to abstract that for users, IMO.

The Issue #203 makes a lot of sense though, because as you see I got stuck and had to make this one.

I see 3 choices:

1. Wrap vuetify Slider in Solara and add Literal['end_points_only']
2. Update the underlying library ipyvuetify to add this additional Type Union with end_points_only or something.
3. Scrap this PR

Using a union-type like I did is something I've seen in multiple libraries, such as transformers by huggingface when you've different options. All to simplify the DX.

I think you should do the choice, but personally I see 1 or 2 as the best ones.

[Lundez]

@maartenbreddels any final decision? :)

[Lundez]

@maartenbreddels I updated into what we talked about, also fixing the range problem you pointed out.

Not sure exactly how I'd build a unit-test if we wish to add that to the repo.

[maartenbreddels]

https://github.com/widgetti/solara/pull/185/files should help as a template

[Lundez]

@maartenbreddels please take a look 👍

Couldn't install dotenv locally so I did some testing using gh actions... 😅

[Lundez]

@maartenbreddels are you back from vacation? :)

[Lundez]

@maartenbreddels thanks for the feedback. I pushed propsed changes and ask if you'll in the future consider changing the defaults :)

[Lundez]

@maartenbreddels both changes are now applied. Sorry for late turn-around :P

[maartenbreddels]

Sorry for late turn-around :P

No problem.

Unit tests are failing because the value we pass to vuetify is not valid it seems.

[Lundez]

I'm not sure I fully understand the error that is in Code Quality. Does it mean that we can't retype variables?

[Lundez]

This test now failed on something else.

================================== FAILURES ===================================
______________________________ test_memoize_hook ______________________________

2 errors was left:

1. Numpy as dependency (I rewrote it manually)
2. You cannot re-type a created variable. I think this is rather poor to not allow (it is configurable in mypy (--allow-redefinition)

[Lundez]

@maartenbreddels the existing failure seems unrelated to my code.

______________________________ test_memoize_hook ______________________________

What should I do`? :)

[Lundez]

@maartenbreddels pinging again XD

[maartenbreddels]

Thank you @Lundez, for your contribution and your patience!

[Lundez]

Hopefully I'll get around to create more PR:s, simply wanted to get the first one done @martinbreddels !

[maartenbreddels]

Released in 1.23.0 🥳