Skip to content

Conversation

@krosenberg
Copy link
Contributor

@krosenberg krosenberg commented Aug 1, 2023

Motivation

  1. When the Slider prop allowTouchTrack is set to true, I expect the center of the thumb to be positioned over my tapped location. Currently, it aligns to the left of the thumb instead of the center.
  2. When allowTouchTrack is false, I can tap anywhere inside the thumb and the thumb position does not change until I begin dragging. But if allowTouchTrack is true, the thumb will immediately left-align itself when I tap anywhere inside the thumb. I expect this behavior of tapping and dragging the thumb to be consistent no matter what the value of allowTouchTrack is.

You can see both issues in the video below. You can reproduce the issue in the Slider examples in the example app.

Before After
rneui_slider_before.mov
rneui_slider_after.mov

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I've manually tested the changed behavior using the example app on an iOS iPhone 14 simulator.

  • Jest Unit Test
  • Checked with example app

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation using yarn docs-build-api
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional context

N/A

@netlify
Copy link

netlify bot commented Aug 1, 2023

Deploy Preview for react-native-elements ready!

Name Link
🔨 Latest commit cab050c
🔍 Latest deploy log https://app.netlify.com/sites/react-native-elements/deploys/64fc39ec82a4de00084c90a2
😎 Deploy Preview https://deploy-preview-3821--react-native-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@krosenberg krosenberg marked this pull request as ready for review August 1, 2023 21:47
@krosenberg
Copy link
Contributor Author

@arpitBhalla anything I can do on my end to get this merged? If you feel it's good to go, are you able to merge it?
Thanks!

@arpitBhalla
Copy link
Member

Hey @krosenberg can you ping me on discord

@codecov
Copy link

codecov bot commented Sep 9, 2023

Codecov Report

Merging #3821 (5a2ec3e) into next (1a983ff) will decrease coverage by 0.22%.
The diff coverage is 6.25%.

❗ Current head 5a2ec3e differs from pull request most recent head cab050c. Consider uploading reports for the commit cab050c to get more accurate results

@@            Coverage Diff             @@
##             next    #3821      +/-   ##
==========================================
- Coverage   79.91%   79.69%   -0.22%     
==========================================
  Files          87       87              
  Lines        1837     1842       +5     
  Branches      809      811       +2     
==========================================
  Hits         1468     1468              
- Misses        364      369       +5     
  Partials        5        5              
Files Changed Coverage Δ
packages/base/src/Slider/Slider.tsx 66.14% <6.25%> (-1.77%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@arpitBhalla arpitBhalla merged commit a47e2f0 into react-native-elements:next Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants