JuliaPluto / PlutoUI.jl

https://featured.plutojl.org/basic/plutoui.jl
The Unlicense
299 stars 54 forks source link

Added width kwarg to Slider #178

Closed Dejan-Ilic closed 1 year ago

Dejan-Ilic commented 2 years ago

I added a keyword argument width to Slider ranging from 0.0 (0% width) to 1.0 (100% width) so users can conveniently resize their sliders. This gives us more fine-grained control. I set the default to 0.2, i.e. 20%, width.

I did not implement any bounds checks, i.e. min(1.0, max(0.0, slider.width)).

Screenshot:

Screenshot 2021-12-30 at 13 58 36
github-actions[bot] commented 2 years ago

Try this Pull Request!

Open Julia and type:

  julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/Dejan-Ilic/PlutoUI.jl", rev="slider-width")
julia> using PlutoUI

Or run this code in your browser: Run with binder

fonsp commented 2 years ago

How about something more general: a style keyword argument, that can be set to (width="100%",) for full width?

See https://juliapluto.github.io/HypertextLiteral.jl/stable/attribute/

Dejan-Ilic commented 2 years ago

Ok I'll try that.

If I understand correctly, you'd like me to add the keyword style to the function Slider, like so: Slider([...], style)

and then a corresponding field to the struct Slider, like so:

struct Slider
    [...]
    style 
end

But I am not sure about the type annotations.

And do you want me to keep my original function with the width keyword and rewrite it as Slider([...] width::String = "20%")? Because Slider([...] style=(width="100%",)) would, in my opinion, be a bit too cumbersome to just enlarge the Slider.

Dejan-Ilic commented 2 years ago

I went ahead and added both a style and a width kwarg, both are more general than before, as demonstrated by this screenshot:

Screenshot 2022-01-06 at 23 09 54

When both style and width are used, width is ignored.

Dejan-Ilic commented 2 years ago

Or if you'd prefer the Slider function as a oneliner using the ternary operator, I could rewrite its body as

Slider(values, (default === missing) ? first(values) : convert(T, default), 
                        show_value, 
                        (style === missing) ? ((width === missing) ? (width="25%",) : (width=width,)) : style)
Dejan-Ilic commented 1 year ago

Sorry this is my first time contributing and I' pretty sure I've messed something up while merging. I'll try again later