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.

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

Fix Slider web offset issue #806

Merged

Conversation

YoussefHenna
Copy link
Collaborator

@YoussefHenna YoussefHenna commented Oct 24, 2023

  • Web version of the slider had an issue where the cursor location did not match the location of the thumb, there was a visible offset between them.
  • Some digging in the code led to realization that the issue is because of some logic that takes place in an onLayout that relies on a ref. Initial call of the onLayout uses an uninitialized ref (as is expected based on React behavior), which leads to an improperly initialized variable that indicates the x position of the slider. The workaround here is to force onLayout to be called again so that it gets initialized properly. See the comment in the code for more details.

@linear
Copy link

linear bot commented Oct 24, 2023

P-4133 Slider on the web view has offset problem

Playground app

Summary : While sliding the thump on the webview, the position of the slider's thump is not matcing the mouse position, you can take a look at the loom video below

https://www.loom.com/share/fa316567e984429e8daf332ac1712f02

@sweep-ai
Copy link

sweep-ai bot commented Oct 24, 2023

Apply Sweep Rules to your PR?

  • Apply: Leftover TODOs in the code should be handled.
  • Apply: All new business logic should have corresponding unit tests in the tests/ directory.
  • Apply: Any clearly inefficient or repeated code should be optimized or refactored.

@github-actions
Copy link

Published version: @draftbit/[email protected]

@github-actions
Copy link

Published version: @draftbit/[email protected]

@YoussefHenna YoussefHenna changed the title Pass in ref to native slider Fix Slider web offset issue Oct 24, 2023
@YoussefHenna YoussefHenna marked this pull request as ready for review October 24, 2023 15:54
@github-actions
Copy link

Published version: @draftbit/[email protected]

Copy link
Contributor

@needs needs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find!

packages/core/src/components/Slider.tsx Show resolved Hide resolved
@YoussefHenna YoussefHenna merged commit 0c7b439 into master Oct 25, 2023
3 checks passed
@YoussefHenna YoussefHenna deleted the youssef/p-4133-slider-on-the-web-view-has-offset-problem branch October 25, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants