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.

Sign up for GitHub

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

Jump to bottom

Blur & mouseup events should be fired ignoring phx-throttle #2318

Closed
tqwewe opened this issue Nov 7, 2022 · 1 comment
Closed

Blur & mouseup events should be fired ignoring phx-throttle #2318

tqwewe opened this issue Nov 7, 2022 · 1 comment
Labels
bug upnext

Comments

@tqwewe
Copy link

tqwewe commented Nov 7, 2022

Environment

  • Elixir version (elixir -v):
    • Erlang/OTP 25 [erts-13.1.1] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [jit:ns] [dtrace]
    • Elixir 1.14.1 (compiled with Erlang/OTP 25)
  • Phoenix version (mix deps): phoenix 1.6.13
  • Phoenix LiveView version (mix deps): phoenix_live_view 0.17.12
  • Operating system: MacOS
  • Browsers you attempted to reproduce this bug on (the more the merrier): Google Chrome, Safari
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: no

Actual behavior

When implementing a slider with phx-throttle="500", the value is not updated if I release the mouse too soon.

Screen.Recording.2022-11-07.at.2.20.08.pm.mov

The min is 100, and max is 1000 in this example. When I click away from the slider, thats when it suddenly snaps back to its old value.

<input
    name="tick_frequency"
    type="range"
    min="100"
    max="1000"
    value="500"
    phx-throttle="500"
    phx-change="ChangeTickFrequency" />

Expected behavior

When I release my mouse, the throttle settings should be ignored, and an event should be fired immediately.
SteffenDE added a commit to SteffenDE/phoenix_live_view that referenced this issue Jan 20, 2024
@SteffenDE
always emit event on blur, even if throttled (phoenixframework#2318)
01f536c
@SteffenDE SteffenDE mentioned this issue Jan 20, 2024
Merged
chrismccord pushed a commit that referenced this issue Jan 24, 2024
@SteffenDE
always emit event on blur, even if throttled (#2318) (#3033)
55d50ef
@chrismccord
Copy link
Member

chrismccord commented Jan 24, 2024 •
edited
Loading

Blur is now triggered. Mouseup has a narrow scope to sliders and should still be treated as a throttle change imo. Thanks!

@DaTrader DaTrader mentioned this issue Feb 2, 2024
Closed
SteffenDE added a commit to SteffenDE/phoenix_live_view that referenced this issue Feb 7, 2024
@SteffenDE
Ensure that events are always sent on blur
f76cc9d 
Relates to: phoenixframework#3033
Relates to: phoenixframework#2318
Fixes: phoenixframework#3076

The old "fix" from phoenixframework#3033 had a fatal flaw because it would call an old
callback on blur.

This new commit adjusts the behavior to properly use the correct callback
by hooking into the cycle logic and ignoring the throttle in case of blur,
leaving the rest of the logic nearly untouched. This requires us to clear
the throttle timeout though, because otherwise we would call the callback
again after a blur, or multiple times when new events happen after blur
but before the next throttle timeout triggers.

The debounce/throttle code is not that easy to understand, but this time
I'm more confident that the solution is actually correct.
(famous last words)
@SteffenDE SteffenDE mentioned this issue Feb 7, 2024
Merged
chrismccord pushed a commit that referenced this issue Feb 8, 2024
@SteffenDE
Ensure that events are always sent on blur, but this time correctly (#…
8cffda7 
…3097)

* Ensure that events are always sent on blur

Relates to: #3033
Relates to: #2318
Fixes: #3076

The old "fix" from #3033 had a fatal flaw because it would call an old
callback on blur.

This new commit adjusts the behavior to properly use the correct callback
by hooking into the cycle logic and ignoring the throttle in case of blur,
leaving the rest of the logic nearly untouched. This requires us to clear
the throttle timeout though, because otherwise we would call the callback
again after a blur, or multiple times when new events happen after blur
but before the next throttle timeout triggers.

The debounce/throttle code is not that easy to understand, but this time
I'm more confident that the solution is actually correct.
(famous last words)

* Always emit the last event when throttling

When throttling events, the last event was discarded, but this is not
what I would expect from a throttled event. Throttling an input should
ensure that it is only emitted at most every `delay` milliseconds, but
it should always emit the last event after the delay has passed. This
change ensures that the last event is always emitted when throttling.

This of course means that events that are emitted shortly after the last
throttled event will still be emmitted immeditately, basically starting
a new throttle window.
SteffenDE added a commit to SteffenDE/phoenix_live_view that referenced this issue Feb 10, 2024
@SteffenDE
undo debounce/throttle changes
3e452ef 
Relates to: phoenixframework#3097
Relates to: phoenixframework#3033
Relates to: phoenixframework#2318
Relates to: phoenixframework#3076

This reverts the changes made in phoenixframework#3097 and phoenixframework#3033. It adds one small
change to the original behavior to clear the throttle timeout on blur.

For cases where the user wants to get the latest value on blur when
throttle is used, adding a `phx-blur={JS.dispatch("change")}` is the
way to go:

```
<input
  name="tick_frequency"
  type="range"
  min="100"
  max="1000"
  value={@tick_frequency}
  phx-throttle="2000"
  phx-change="change-tick-frequency"
  phx-blur={JS.dispatch("change")}
/>
```
SteffenDE added a commit to SteffenDE/phoenix_live_view that referenced this issue Feb 10, 2024
@SteffenDE
undo debounce/throttle changes
9cfcd33 
Relates to: phoenixframework#3097
Relates to: phoenixframework#3033
Relates to: phoenixframework#2318
Relates to: phoenixframework#3076

This reverts the changes made in phoenixframework#3097 and phoenixframework#3033. It adds one small
change to the original behavior to clear the throttle timeout on blur.

For cases where the user wants to get the latest value on blur when
throttle is used, adding a `phx-blur={JS.dispatch("change")}` is the
way to go:

```
<input
  name="tick_frequency"
  type="range"
  min="100"
  max="1000"
  value={@tick_frequency}
  phx-throttle="2000"
  phx-change="change-tick-frequency"
  phx-blur={JS.dispatch("change")}
/>
```
SteffenDE added a commit to SteffenDE/phoenix_live_view that referenced this issue Feb 10, 2024
@SteffenDE
undo debounce/throttle changes
524d0da 
Relates to: phoenixframework#3097
Relates to: phoenixframework#3033
Relates to: phoenixframework#2318
Relates to: phoenixframework#3076

This reverts the changes made in phoenixframework#3097 and phoenixframework#3033. It adds one small
change to the original behavior to clear the throttle timeout on blur.

For cases where the user wants to get the latest value on blur when
throttle is used, adding a `phx-blur={JS.dispatch("change")}` is the
way to go:

```
<input
  name="tick_frequency"
  type="range"
  min="100"
  max="1000"
  value={@tick_frequency}
  phx-throttle="2000"
  phx-change="change-tick-frequency"
  phx-blur={JS.dispatch("change")}
/>
```
@SteffenDE SteffenDE mentioned this issue Feb 10, 2024
Merged
SteffenDE added a commit to SteffenDE/phoenix_live_view that referenced this issue Feb 10, 2024
@SteffenDE
undo debounce/throttle changes
8a6f06a 
Relates to: phoenixframework#3097
Relates to: phoenixframework#3033
Relates to: phoenixframework#2318
Relates to: phoenixframework#3076

This reverts the changes made in phoenixframework#3097 and phoenixframework#3033. It adds one small
change to the original behavior to clear the throttle timeout on blur.

For cases where the user wants to get the latest value on blur when
throttle is used, adding a `phx-blur={JS.dispatch("change")}` is the
way to go:

```
<input
  name="tick_frequency"
  type="range"
  min="100"
  max="1000"
  value={@tick_frequency}
  phx-throttle="2000"
  phx-change="change-tick-frequency"
  phx-blur={JS.dispatch("change")}
/>
```
chrismccord pushed a commit that referenced this issue Feb 14, 2024
@SteffenDE
undo debounce/throttle changes (#3108)
f274943 
Relates to: #3097
Relates to: #3033
Relates to: #2318
Relates to: #3076

This reverts the changes made in #3097 and #3033. It adds one small
change to the original behavior to clear the throttle timeout on blur.

For cases where the user wants to get the latest value on blur when
throttle is used, adding a `phx-blur={JS.dispatch("change")}` is the
way to go:

```
<input
  name="tick_frequency"
  type="range"
  min="100"
  max="1000"
  value={@tick_frequency}
  phx-throttle="2000"
  phx-change="change-tick-frequency"
  phx-blur={JS.dispatch("change")}
/>
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug upnext
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@chrismccord @tqwewe