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

refactor(range): update value on touchEnd or drag #29005

Merged
merged 11 commits into from Mar 6, 2024
Merged

refactor(range): update value on touchEnd or drag #29005

merged 11 commits into from Mar 6, 2024

Conversation

thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Feb 8, 2024

Issue number: resolves #28487


What is the current behavior?

There are two behaviors that need to be addressed.

  1. The range value is updated when the gesture onStart event is fired.
    This can lead to the values being accidentally updated when the user is scrolling on the view.
    The user might tap on the range to scroll on the view, but the range value is updated instead.

  2. The component prevents the view from scrolling while the user has touched any part of the range.
    The user might want to scroll and they happen to touch the range. This can lead to the user feeling disoriented because they can't scroll on the view anymore.

These behaviors do not follow the native behavior of mobile devices.

What is the new behavior?

  • The range value is updated on touch end or when the knob is being dragged.
  • The view can be scrolled while the user is not dragging the knob.
  • A new variable isScrollingView is used to determine if the user is scrolling on the view regardless of whether the user is dragging the knob or not. This determines what logic to apply.
  • The pressedKnob variable is no longer being set in the onStart event. It is now being set in the onMove and onEnd events. (the reason behind this can be found within the newly added comments)
  • The initialContentScrollY variable is no longer being set in the onStart event. It is now being set in the onMove event. (the reason behind this can be found within the newly added comments)

I did not change the behavior of the range when the user is dragging the knob. The view should not scroll while the user is dragging the knob.

Does this introduce a breaking change?

  • Yes
  • No

Other information

The new behavior aligns with the native mobile devices.

@github-actions github-actions bot added the package: core @ionic/core package label Feb 8, 2024
@thetaPC thetaPC marked this pull request as ready for review February 15, 2024 18:42
@thetaPC thetaPC requested a review from a team as a code owner February 15, 2024 18:42
@thetaPC thetaPC requested review from amandaejohnston and removed request for a team February 15, 2024 18:42
Copy link
Contributor

@amandaejohnston amandaejohnston left a comment

Choose a reason for hiding this comment

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

Great work on this! I tested the behavior against my findings in #28487 (comment) and everything looks great. Just noticed a thing with the events.

core/src/components/range/range.tsx Show resolved Hide resolved
core/src/components/range/range.tsx Outdated Show resolved Hide resolved
@thetaPC thetaPC added this pull request to the merge queue Mar 6, 2024
Merged via the queue into main with commit 84f5def Mar 6, 2024
46 checks passed
@thetaPC thetaPC deleted the FW-5554 branch March 6, 2024 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: ion-range should not update value or lock scrolling until touchEnd or drag within the bar
2 participants