JuliaPluto / PlutoUI.jl

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

Add update_on_release option to Slider #268

Closed holomorphism closed 7 months ago

holomorphism commented 9 months ago

Add update_on_release option to Slider. (Implement #267)

If this option is set to true, slider value changes will not take effect until the mouse button is released. (The confirm() function is available for the similar purpose, but I thought it would be nice to have this option as well, since it is handy.)

I have checked this to work in the following environment:

In order to support touch input, both "mouseup" and "touchend" events are watched by addEventListener() in the javascript code. (The "mouseup" and "touchend" events did not seem to occur at the same time.)

github-actions[bot] commented 9 months ago

Try this Pull Request!

Open Julia and type:

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

Or run this code in your browser: Run with binder

aplavin commented 9 months ago

Thanks, works fine in simple cases! But I noticed that it behaves strangely when inside combine: an update is always triggered when dragging the mouse, even though the value stays the same. Video: https://github.com/JuliaPluto/PlutoUI.jl/assets/687995/5fcd019d-bb1c-43ed-a67e-941ab3af2bff

holomorphism commented 9 months ago

@aplavin Thanks, I hadn't noticed the combine() problem. I have fixed the points you mentioned.

fonsp commented 9 months ago

You could also make a “debouncer” wrapping widget! (Search for “throttle vs debounce”) That would be really useful in a general way.

Op 21 sep. 2023 om 07:31 heeft holomorphism @.***> het volgende geschreven:

 @aplavin Thanks, I hadn't noticed the combine() problem. I have fixed the points you mentioned.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

holomorphism commented 9 months ago

@fonsp Thanks for the reply. I see you are referring to issue #69. Actually I hadn't tried it yet because the code looked a bit difficult, but I pasted it into my notebook and ran it and it worked wonderfully.

Certainly the "debouncer" looks very useful for reactive programming in Pluto in general. What is missing for the debounced() in #69 to be included in PlutoUI?

fonsp commented 7 months ago

Thanks @holomorphism and sorry for the late reply! What's still missing for #69 is proper support for AbstractPlutoDingetjes. Take a look at how confirm was implemented to see what I mean.

Do you think we should still include this PR if we have a general debounced?

holomorphism commented 7 months ago

Thanks for the reply, @fonsp! I will check the confirm implementation.

Do you think we should still include this PR if we have a general debounced?

I have been using debouced for a while, but for my purposes it seems that this PR is unnecessary if debouced is available. So I am closing this PR.

Thank you very much!

aplavin commented 7 months ago

For those who stumble across this PR and want this feature: Slider(on_release=true) is available in PlutoUIExtra.jl and will remain so. Thanks @holomorphism for the implementation!

fonsp commented 7 months ago

It looks like this could be implemented in a slightly more robust way by listening to the "change" event instead of "input" on the <input type=range> element. That means that you don't need the mouseup handlers, but you still need a wrapper element, custom value property etc.