Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

added swipe support #6

Closed
wants to merge 4 commits into from
Closed

Conversation

netanelgilad
Copy link

Added swipe support for the drawer.
Can be swiped out of the side based on the edge swipe sensitivity.
When drawer is open can be swiped out anywhere.

@spacerobot5
Copy link

Works great for me!

@dfreedm
Copy link
Contributor

dfreedm commented Jul 7, 2014

Can you rebase this to have the original two space tabbing?

@netanelgilad
Copy link
Author

updated.

@IntranetFactory
Copy link

I've tested the latest version of the PR. It broke vertical touch scrolling of main on both ios and android for me.

I changed line 132 (https://github.com/netanelgilad/core-drawer-panel/blob/master/core-drawer-panel.html#L132) to:
this.setAttribute('touch-action', 'pan-y');

and now it works. Swipe left/right controls the drawer, and swipe up/down scrolls main.

@netanelgilad
Copy link
Author

Good catch. I did a few tests on scrolling the drawer when it overflows and I've added a bit more logic to make it work correctly. I've tested it on my android and it seems to work fine. Would love to see a test on ios.

@squeral
Copy link

squeral commented Jul 11, 2014

I added your changes to my project and it works great until you swipe the drawer vertically and then swipe left to close. It doesn't call "on-trackStart" or "on-track".

@netanelgilad
Copy link
Author

@squeral Do you mean that you scroll vertically and while holding your finger down you swipe left and it doesn't close?
I looked at the gmail app for android that has the same drawer idea and it doesn't also swipe left while scrolling vertically. I guess that if the user wants to swipe the drawer out he would lift the finger to start a new gesture.. What do you think?

@squeral
Copy link

squeral commented Jul 11, 2014

No. The behaviour your describing seams right to me. But by issue is when I swipe vertically, lift my finger and swipe horizontally (new gesture) nothing happens.

It works if I tap somewhere before swiping horizontally.
If you can't replicate my issue then I must have something wrong somewhere else in the code.

@netanelgilad
Copy link
Author

I've created a gist of the demo that I used to check the vertical scrolling.
https://gist.github.com/netanelgilad/f0592736c8d55a576d39
Try it out and tell me if you have problems with it. By the way on what platform have you tried it?

@squeral
Copy link

squeral commented Jul 11, 2014

I got the same issue with your gist.
Steps:

  1. tap "toggle drawer"
  2. scroll down inside the drawer
  3. swipe left to close (does nothing!)

If I tap somewhere and swipe again it works...
I'm using Chrome on android.

@netanelgilad
Copy link
Author

Wierd.. I can't seem to replicate this.
But it did help me notice another issue when main in scrolled down the scrim doesn't cover the whole div, so I fixed that. (It seems that it doesn't work in the original as well).

@frankiefu
Copy link
Contributor

In narrow mode, when I tried to drag out the drawer a little bit and then released, the drawer slid back immediately first and then slid out.

BTW, thanks for working on this feature.

@netanelgilad
Copy link
Author

I fixed the issue with the drawer sliding back before opening.
I did notice the issue that @squeral was talking about but I can't figure out why would that happen. Can anyone test it on another platform other than android or windows? I would like to know if it something specific for android cause on my touch enabled windows it doesn't happen...

@frankiefu
Copy link
Contributor

I haven't looked at the code but I have noticed 2 issues when I tried the demo with the changes.

  1. there is no transition when swipe out the drawer from the side.
  2. run the demo in narrow layout, then resize the window to wide layout, the drawer doesn't appear.

frankiefu added a commit that referenced this pull request Jul 17, 2014
#6), fixes #4
- use 'rightDrawer' attribute instead of 'right-drawer' class to make
the drawer position to the right.
@frankiefu
Copy link
Contributor

I went ahead and fixed the issues I mentioned in my previous comment and manually merged your swipe support changes. I also simplified and refactored a few things in the code. I added a comment in the commit and in the code referencing this pull request as the original code for the swipe support. Thanks for the good work.

@frankiefu frankiefu closed this Jul 17, 2014
@squeral
Copy link

squeral commented Jul 18, 2014

I tested it with my content and it works great!! Well done ;)

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

Successfully merging this pull request may close these issues.

6 participants