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

Allow toggling with a swipe gesture, fixes #49 #85

Merged
merged 1 commit into from
Sep 9, 2017

Conversation

lolmaus
Copy link
Collaborator

@lolmaus lolmaus commented Sep 7, 2017

Implemented toggling with a swipe gesture. Fixes #49.

Notes:

  1. I used ember-gestures for finger gestures support.
  2. I used simple panning instead of actual handle dragging because the logic behind dragging would be way more complicated. You can have an idea from this prototype. The gotcha is that with panning toggling happens immediately and not after mouse/finger release.
  3. ember-gestures needs an update of ember-hammertime to support Ember 2.15+. PR pending. This PR currently uses a feature branch of ember-gestures. You can try it out locally, but don't merge this yet.
  4. Animation direction of the skewed styles inverted for consistency with all other styles. This makes a difference because swiping is directional.
  5. Fixed a bug with skewed active state: on Chrome Desktop with mouse clicking it didn't work, on Chrome Android with finger tapping/swiping the toggle stayed active (and misaligned) indefinitely, until user tapped elsewhere.

@lolmaus
Copy link
Collaborator Author

lolmaus commented Sep 7, 2017

Something weird in Travis. Can you please try clearing all caches and restarting the build?

@knownasilya
Copy link
Owner

Cleared and restarted

@knownasilya
Copy link
Owner

knownasilya commented Sep 7, 2017

Looks like we need to update the version of node for travis, probably to 6.x. See if changing that fixes this issue.

@eriktrom
Copy link

eriktrom commented Sep 8, 2017

fyi - blocked by html-next/ember-gestures#102 fyi

@lolmaus
Copy link
Collaborator Author

lolmaus commented Sep 8, 2017

@vvainio has brought my attention to the fact that my initial implementation does not support mouse gestures, only touch. This was probably due to the fact that ember-gestures has a minimum stroke length for the swipe gesture which is rarely achieved with a mouse.

I've changed swipe to pan, so now both mouse and finger gestures behave consistently.

This revealed a gotcha: when you pan with a mouse and release the mouse button over the toggle, a click even is produced on the <label> element, which immediately undoes toggling. I came up with a witty workaround which resolves the problem without interfering with a11y.

@knownasilya I've bumped Node and it didn't help. Lets wait for html-next/ember-gestures#102 to finalize, it may resolve the issue.

@webark
Copy link
Contributor

webark commented Sep 8, 2017

@lolmaus I'm currently using this in a project without jquery. Is there anyway we could handle this without having to use it?

@lolmaus
Copy link
Collaborator Author

lolmaus commented Sep 8, 2017

@webark Granted.

@webark
Copy link
Contributor

webark commented Sep 8, 2017

🙏

@eriktrom
Copy link

eriktrom commented Sep 8, 2017

ember-gestures fixed upstream, released as 0.4.7, fyi

@lolmaus
Copy link
Collaborator Author

lolmaus commented Sep 9, 2017

@eriktrom Thank you. 🙇 PR updated.

Copy link
Owner

@knownasilya knownasilya left a comment

Choose a reason for hiding this comment

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

Just some small changes, then we should be able to merge 👍

mouse button is released.
*/
_disableLabelUntilMouseUp () {
if (this.get('labelDisabled')) return;
Copy link
Owner

Choose a reason for hiding this comment

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

Could you make this a multi line if with { .. }.


document.addEventListener('mouseup', () => {
next(() => this.set('labelDisabled', false));
})
Copy link
Owner

Choose a reason for hiding this comment

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

Missing semicolon

@lolmaus lolmaus changed the title [blocked] Allow toggling with a swipe gesture, fixes #49 Allow toggling with a swipe gesture, fixes #49 Sep 9, 2017
@lolmaus
Copy link
Collaborator Author

lolmaus commented Sep 9, 2017

@knownasilya I've addressed your comments and squashed.

@knownasilya knownasilya merged commit 398f8d6 into knownasilya:master Sep 9, 2017
@knownasilya
Copy link
Owner

Published as 5.1.0

@lolmaus
Copy link
Collaborator Author

lolmaus commented Sep 9, 2017

@knownasilya Thank you, comrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants