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(toggle): swipe gesture applies to knob #27255

Merged
merged 3 commits into from
Apr 26, 2023
Merged

fix(toggle): swipe gesture applies to knob #27255

merged 3 commits into from
Apr 26, 2023

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Apr 20, 2023

Issue number: resolves #27254


What is the current behavior?

The swipe gesture is currently applied to the entire ion-toggle element. This was fine for the legacy syntax, but with the modern syntax it means users can swipe on the label text which is not correct.

What is the new behavior?

  • The toggle now creates the gesture on the .toggle-icon element which is the container for the knob for both modern and legacy syntaxes.
  • Moved touch-action: none to the host of the legacy toggle. This was preventing scrolling from happening on the modern toggle. I double checked with native iOS and you can scroll when a pointer moves over a toggle.

The structure of this fix was designed to match what ion-range does:

private setupGesture = async () => {
const rangeSlider = this.rangeSlider;
if (rangeSlider) {
this.gesture = (await import('../../utils/gesture')).createGesture({
el: rangeSlider,
gestureName: 'range',
gesturePriority: 100,
threshold: 0,
onStart: (ev) => this.onStart(ev),
onMove: (ev) => this.onMove(ev),
onEnd: (ev) => this.onEnd(ev),
});
this.gesture.enable(!this.disabled);
}
};

Modern Legacy
Screen.Recording.2023-04-20.at.12.34.53.PM.mov
Screen.Recording.2023-04-20.at.12.35.19.PM.mov

Does this introduce a breaking change?

  • Yes
  • No

Other information

@stackblitz
Copy link

stackblitz bot commented Apr 20, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions bot added the package: core @ionic/core package label Apr 20, 2023
@@ -240,8 +240,6 @@ input {

background: var(--track-background);

pointer-events: none;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is removed from the .toggle-icon element so users can swipe on the element.

@liamdebeasi liamdebeasi marked this pull request as ready for review April 20, 2023 16:54
@liamdebeasi liamdebeasi requested a review from a team as a code owner April 20, 2023 16:54
@liamdebeasi
Copy link
Contributor Author

I added another part of the fix so re-requesting reviews:

Moved touch-action: none to the host of the legacy toggle. This was preventing scrolling from happening on the modern toggle. I double checked with native iOS and you can scroll when a pointer moves over a toggle.

See 656526f

@lincolnthree
Copy link

Dev build works great in my testing.

Copy link
Contributor

@sean-perkins sean-perkins left a comment

Choose a reason for hiding this comment

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

Nice work 👍

@liamdebeasi liamdebeasi added this pull request to the merge queue Apr 26, 2023
Merged via the queue into main with commit 6524582 Apr 26, 2023
@liamdebeasi liamdebeasi deleted the FW-4108 branch April 26, 2023 15:18
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-toggle and rendered label blocks vertical gestures/touch-scrolling
3 participants