fix(slider): make delta calculation idempotent#205
Open
matta wants to merge 1 commit intoDioxusLabs:mainfrom
Open
fix(slider): make delta calculation idempotent#205matta wants to merge 1 commit intoDioxusLabs:mainfrom
matta wants to merge 1 commit intoDioxusLabs:mainfrom
Conversation
The Slider component tracks dragging via a reactive effect, call it the drag effect. Previously, the delta was read from a global `Pointer` struct every time the drag effect ran. The bug: the drag effect was not idempotent, and could run more than once, for reasons outside the Slider's control. It could apply a pointer event's "delta" to the Slider multiple times. Depending on the way the app was written, this could cause the Slider value to deviate far from the what it should be while the user dragged. This change fixes the bug by shifting the delta calculation inside the drag effect. By introducing a local `use_hook` to track the last processed position, the effect becomes idempotent with respect to the most recently observed pointer position.
|
Preview available at https://dioxuslabs.github.io/components/pr-preview/pr-205/ |
Contributor
Author
|
I ran into this bug because my app signals a bunch of state change as sliders move, which I suppose was the cause of the Slider's use_effect being re-run multiple times per pointer event in some cases. I'm a web-dev and Dioxus noob so I have no claim to this commit being the best way to fix it. The issue is tricky enough that I'm not sure what kind of test hygiene you'd expect as maintainers in this case. I tested the fix against my app and it fixed things nicely. Let me know if I should change anything. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Slider component tracks dragging via a reactive effect, call it the drag effect. Previously, the delta was read from a global
Pointerstruct every time the drag effect ran.The bug: the drag effect was not idempotent, and could run more than once, for reasons outside the Slider's control. It could apply a pointer event's "delta" to the Slider multiple times. Depending on the way the app was written, this could cause the Slider value to deviate far from the what it should be while the user dragged.
This change fixes the bug by shifting the delta calculation inside the drag effect. By introducing a local
use_hookto track the last processed position, the effect becomes idempotent with respect to the most recently observed pointer position.