New property to force swiping in a single direction #5

leodido · 2016-07-19T16:35:01Z

Referring to discussion in #4 I added a new property - i.e. single-direction to force the swiping to work only in the same direction eventually specified by swipeLeft or swipeRight property.

When single-direction property is set on <paper-swipe> the component can only swipe towards the edge chosen.

This PR does NOT introduce any breaking change.

I also added two demo blocks.

When single-direction property is set the component can only swipe towards the edge chosen.

… edge) Available ony when `singleDirection` property is set. Swipeable content can swipe up to its initial position (i.e. the right edge of its container when `swipeLeft` is set, the left one when `swipeRight` is set).

motss · 2016-07-20T11:45:24Z

@leodido So basically you want the following behavior, am I right?

_New behavior:_

`swipeLeft`	`swipeRight`	Expected result
0	0	no swiping is allowed (which is equivalent to `disableSwipe` is set
0	1	ONLY swipe to the right
1	0	ONLY swipe to the left
1	1	Can be swiped to either side

where 0 equates false while 1 equates true

_Old (or current) behavior:_

`swipeLeft`	`swipeRight`	Expected result
0	0	Can be swiped to either side
0	1	ONLY swipe to the right
1	0	ONLY swipe to the left
1	1	No support for this

where 0 equates false while 1 equates true

leodido · 2016-07-20T14:53:32Z

Hi @motss, not exactly.

I think it is correct that this component always swipe (except when user explicitly disables it), since it's called <paper-swipe>.

Simply it should also be able to constrain the swiping direction (1st aspect). Indeed now there is not a way to disallow entirely the swiping in the opposite direction. This was the reason for the introduction of the singleDirection property.

In #4 I was referring to the meaning of swipeLeft and swipeRight properties. Infact they currently simply refer to the edge to which the swipeable container pins at when swiping (2nd aspect).

I think this is the current behavior.

swipeLeft	swipeRight	Container edge to which it pins at	Swipes to left	Swipes to right
0	1	right	yes	yes and pins
0	0	both	yes and pins	yes and pins
1	0	left	yes and pins	yes
1	1	both	yes and pins	yes and pins

Rather the behavior I'm trying to introduce (and to complete) with this PR is.

Assuming singleDirection is not set (i.e. false).

leftPeek	rightPeek	Container edge to which it pins at	Swipes to left	Swipes to right
0	1	right	yes	yes and pins
0	0	both	yes and pins	yes and pins
1	0	left	yes and pins	yes
1	1	both	yes and pins	yes and pins

Assuming singleDirection is set (i.e. true).

leftPeek	rightPeek	Container edge to which it pins at	Swipes to left	Swipes to right
0	1	right	until reaches the left edge	yes and pins
0	0	both	yes and pins (`singleDirection` not applicable)	yes and pins (`singleDirection` not applicable)
1	0	left	yes and pins	until reaches the right edge
1	1	both	yes and pins (`singleDirection` not applicable)	yes and pins (`singleDirection` not applicable)

What do you think about?

If you agree I could proceed with another commit.

motss · 2016-07-20T16:28:27Z

@leodido I like the idea of disallowing the element to swipeable element to left when swipeRight is set and vice versa. As you know, the current one will move the swipeable element back to its original position while you're dragging it to the opposite direction. That is totally intended to indicate that you're not allowed to swipe to the opposite direction no matter how hard you try to but the current outcome is like what you mentioned (you can swipe to both direction but the swipeable element is pinned at, say right when swipeRight is set).

This is my proposal and feel free to leave comments:

Change the name of singleDirection to noOppositeSwipe (is this better?)
Instead of having another method _directionAllowed, maybe we can modify somewhere so that you can still swipe to the opposite direction but you don't see the swipeable element moving at all as the swipeable element has been restricted to just one direction, left or right. Your approach works fine except you didn't account for the situation where the user might want to swipe to the opposite direction first. When swipeRight is set, you try to swipe in the opposite direction first which is left then you will soon realize that you can no longer swipe to the right unless you trigger the _upHandler event to release tracking. In short, users are allowed to swipe to either direction but the drawer is allowed to move in ONLY one direction. This might be a better one:

swipeRight	swipeLeft	noOppositeSwipe	swiping direction allowed	swipeable element
0	0	x	both	swipe to either side and pin, `noOppositeSwipe` does not apply here.
0	0	x	both	swipe to either side and pin, `noOppositeSwipe` does not apply here.
0	1	0	both	swipe to left and pin, but can be dragged to the right with a maximum distance defined by `slideOffset` and back to original position once released.
0	1	1	both	swipe to left and pin, cannot move even a bit in the opposite direction.
1	0	0	both	swipe to right and pin, but can be dragged to the left with a maximum distance defined by `slideOffset` and back to original position once released.
1	0	1	both	swipe to right and pin, cannot move even a bit in the opposite direction.
1	1	x	both	swipe to either side and pin, `noOppositeSwipe` does not apply here.
1	1	x	both	swipe to either side and pin, `noOppositeSwipe` does not apply here.

If possible, please help me to remove the iron-resizable-behavior import, iron-resize event listener, and the _resize method.

…equivalent to the default behavior (i.e. both properties missing)

leodido · 2016-07-20T17:10:36Z

Agree for noOppositeSwipe, very good name.
Get rid of IronResizableBehavior
Locally I removed IDE files (i.e., .idea/ commited folder) and inserted them into .gitignore. I think they should not be in the repo. Can I commit this change?
Since the track event is on the host and not on the swipeable content I think we should restrict it. E.g., when dragging/swiping on the orange part (see demo) the drawer should not respond. Locally I've a working solution. If you agree I commit it.
Regarding your table. If I not misunderstood it, beyond little details, we perfectly have same idea. Here we are!
Your idea of slideOffset is a good one ..
~~Yes _upHandler should be triggered.~~ You're right.

The track event must be not ignored when user drags (without releasing pointer) in the allowed direction after having dragged in the opposite and disallowed direction.

motss · 2016-07-20T17:15:38Z

Locally I removed IDE files (i.e., .idea/ commited folder) and inserted them into .gitignore. I think they should not be in the repo. Can I commit this change?

Much appreciated. Thanks.

Since the track event is on the host and not on the swipeable content I think we should restrict it. E.g., when dragging/swiping on the orange part (see demo) the drawer should not respond. Locally I've a working solution. If you agree I commit it.

Worth taking a look.

…pe in the opposite direction This commit fixes following bug. 1) swipeLeft, noOppositeSwipe are set 2) user tries to swipe towards right (resulting in a not allowed swipe movement) 3) user, without releasing the pointer, tries to swipe in the allowed direction 4) swipe does not work (also if in the allowed direction) until pointer is released This commit ensures the user can swipe back also without releasing the pointer, when the pointer comes back ans passes its first position.

leodido · 2016-07-21T15:42:06Z

Hi @motss I think here we are. What do you think?

We can posticipate you good idea about slideOffset.

P.S.: Probably, to fire a opposite-edge event make sense.
P.P.S.: Also investigating a debounce on (some or all event handlers) can make sense.

motss · 2016-07-21T17:22:13Z

@leodido Thanks for the update.

~~Yes _upHandler should be triggered.~~ You're right.

The track event must be not ignored when user drags (without releasing pointer) in the allowed direction after having dragged in the opposite and disallowed direction.

Steps to repro this issue (Assuming on a touch device):

Create a paper-swipe and set swipeLeft and noOppositeSwipe to true.
A paper-swipe that is allowed to be swiped to the left is created and it is not allowed to move towards the right at all.
Drag the paper-swipe to the right (which is the opposite direction) without lifting your finger up then drag it to the left and see if there is any movement. If no, this behavior is incorrect as the paper-swipe cannot be moved at all at this moment even though it's still in the tracking stage not yet trackend.
Finger off the screen then try dragging it to the left then to right and see if any difference. (The paper-swipe should be able to move left and right without the finger leaving the screen and this is the correct behavior).

motss · 2016-07-21T17:26:03Z

P.S.: Probably, to fire a opposite-edge event make sense.

Why is this needed?

P.P.S.: Also investigating a debounce on (some or all event handlers) can make sense.

Is it because of performance issue?

leodido · 2016-07-21T17:58:36Z

Why is this needed?

Could be useful for the user to detect when _curPos is null/0 ?

Is it because of performance issue?

Yes.

Steps to repro this issue (Assuming on a touch device):

Create a paper-swipe and set swipeLeft and noOppositeSwipe to true.

A paper-swipe that is allowed to be swiped to the left is created and it is not allowed to move towards the right at all.

Drag the paper-swipe to the right (which is the opposite direction) without lifting your finger up then drag it to the left and see if there is any movement. If no, this behavior is incorrect as the paper-swipe cannot be moved at all at this moment even though it's still in the tracking stage not yet trackend.

Then try dragging it to the left then to right and see if any difference. (The paper-swipe should be able to move left and right without the finger leaving the screen and this is the correct behavior).

I cannot repro the issue ... It is not clear to me, please help/explain.
The only "limitation" I found is:
User tries to swipe towards right, suppose event.detail.dx (distance from first track event pos) is 22, until the event.detail.dx is -22 (user is now dragging towards left), the handler does not start. When event.detail.dx is -23 is moves towards left.

motss · 2016-07-22T06:44:38Z

paper-swipe.html

+        var hover = event.detail.hover();
+        var isOverContent = hover && Polymer.dom(this._content).deepContains(hover);
+
+        if (this._swipeAllowed() && isOverContent) {


@leodido Why hover is needed?

@motss isOverContent is needed to avoid that the swiping works also on the underlay part, this way it only the swipeable content responds to track events.
If you do not like it by default we can make it a opt-in or opt-in behavior with a boolean property.

@leodido Appreciated your effort. To avoid confusion, I suggest making this change as a separate issue. Currently this PR aims to introduce new property to eliminate swipe in opposite direction. WDYT?

Right. I will move this change to another branch/PR.

Removed from current PR.

motss · 2016-07-22T06:45:55Z

The only "limitation" I found is:
User tries to swipe towards right, suppose event.detail.dx (distance from first track event pos) is 22, until the event.detail.dx is -22 (user is now dragging towards left), the handler does not start. When event.detail.dx is -23 is moves towards left.

Is this limitation found before or after this PR?

I cannot repro the issue ... It is not clear to me, please help/explain.
I already tried my best to explain though. Let's give it another try.

This element as the name implied is able to swipe to either side of the screen and it goes off the screen and never come back or disappear just like any swipeable cards or notifications that you can probably find on iOS and Android.

To add more functionalities for the user to customize the swipeable behavior, there are a few things introduced:

slideOffset - the minimum distance you swipe to make the swipeable element to move itself all the way to the edge of the screen and it can go off the screen or show a portion of itself from the edge defined by peekOffset.
swipeLeft and swipeRight to limit the direction that swipeable element can be swiped to. However, the swipeable element will kind of bounce back to its original position while swiping in the opposite direction like so:

A few things aim to fix in this PR:

Add a new property noOppositeSwipe to totally eliminate the bounce back effect while swiping in the opposite direction.
Limit the distance of the swipeable element can move while swiping in the opposite direction using slideOffset so that a) it can preserve the bounce back effect and b) to correct the behavior as the current one user can actually swipe the entire thing off the screen but it will come back later on which get the user into thinking that why can't it swipe in that direction.

leodido · 2016-07-27T09:08:00Z

Hey @motss I'm back.

So, in summary what this PR misses is:

Limit the distance the swipeable content can move towards the opposite direction to slideOffset.

It is correct?

Some considerations on this topic.
The property slideOffset refers to the numbers of pixels needed to trigger auto-slide to the edge.
Why to use it also to limit the bounce in the opposite direction?
In this regard, we may introduce a second property (e.g., oppositeBounceOffset or simply bounceOffset) to give the user the ability to handle exclusively this concept. What do you think?
Furthermore: the bounce effect in the case of noOppositeSwipe is set should be restricted to the opposite direction or also to the primary direction?

…e direction to slideOffset

motss · 2016-07-27T11:37:14Z

slideOffset is adequate. By default, we need to drag at a distance beyond the value of slideOffset and release the draggable panel. It will slide all its way to the edge. So we should limit the distance in the opposite direction instead of allowing users to drag it beyond the slideOffset if noOppositeSwipe is true.

Furthermore: the bounce effect in the case of noOppositeSwipe is set should be restricted to the opposite edge or also to the primary edge?
Opposite edge? primary edge?

leodido · 2016-07-27T12:30:20Z

Hey @motss, have you seen the last commit?

leodido · 2016-08-01T01:03:12Z

\ping @motss

motss · 2016-08-02T06:15:22Z

@leodido Might need some time to review.

leodido · 2016-09-29T09:56:46Z

ping @motss

New property to force swiping in a single direction

bab944e

When single-direction property is set the component can only swipe towards the edge chosen.

leodido force-pushed the feature/single-direction branch from 4b0b724 to bab944e Compare July 20, 2016 01:34

motss added enhancement help wanted bug labels Jul 20, 2016

leodido added 2 commits July 20, 2016 18:54

When both properties (swipeLeft, swipeRight) are set the behavior is …

b2f8936

…equivalent to the default behavior (i.e. both properties missing)

Get rid of unused IronResizableBehavior

d9a9ac5

leodido added 3 commits July 20, 2016 19:17

Property singleDirection renamed noOppositeSwipe

d01b0d4

Better ignore list plus cleaning-up of settings files

e2d30fe

motss reviewed Jul 22, 2016
View reviewed changes

motss added dont-merge-just-yet good-first-bug in-progress review-needed and removed bug labels Jul 22, 2016

leodido force-pushed the feature/single-direction branch from e34af9e to e2d30fe Compare July 27, 2016 09:01

Limit the distance the swipeable content can move towards the opposit…

2efbb98

…e direction to slideOffset

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New property to force swiping in a single direction #5

New property to force swiping in a single direction #5

leodido commented Jul 19, 2016 •

edited

motss commented Jul 20, 2016 •

edited

leodido commented Jul 20, 2016

motss commented Jul 20, 2016 •

edited

leodido commented Jul 20, 2016 •

edited

motss commented Jul 20, 2016

leodido commented Jul 21, 2016 •

edited

motss commented Jul 21, 2016 •

edited

motss commented Jul 21, 2016

leodido commented Jul 21, 2016 •

edited

motss Jul 22, 2016

leodido Jul 22, 2016 •

edited

motss Jul 22, 2016 •

edited

leodido Jul 22, 2016

leodido Jul 27, 2016

motss commented Jul 22, 2016

leodido commented Jul 27, 2016 •

edited

motss commented Jul 27, 2016

leodido commented Jul 27, 2016 •

edited

leodido commented Aug 1, 2016

motss commented Aug 2, 2016

leodido commented Sep 29, 2016

New property to force swiping in a single direction #5

Are you sure you want to change the base?

New property to force swiping in a single direction #5

Conversation

leodido commented Jul 19, 2016 • edited

motss commented Jul 20, 2016 • edited

leodido commented Jul 20, 2016

motss commented Jul 20, 2016 • edited

leodido commented Jul 20, 2016 • edited

motss commented Jul 20, 2016

leodido commented Jul 21, 2016 • edited

motss commented Jul 21, 2016 • edited

motss commented Jul 21, 2016

leodido commented Jul 21, 2016 • edited

motss Jul 22, 2016

Choose a reason for hiding this comment

leodido Jul 22, 2016 • edited

Choose a reason for hiding this comment

motss Jul 22, 2016 • edited

Choose a reason for hiding this comment

leodido Jul 22, 2016

Choose a reason for hiding this comment

leodido Jul 27, 2016

Choose a reason for hiding this comment

motss commented Jul 22, 2016

leodido commented Jul 27, 2016 • edited

motss commented Jul 27, 2016

leodido commented Jul 27, 2016 • edited

leodido commented Aug 1, 2016

motss commented Aug 2, 2016

leodido commented Sep 29, 2016

leodido commented Jul 19, 2016 •

edited

motss commented Jul 20, 2016 •

edited

motss commented Jul 20, 2016 •

edited

leodido commented Jul 20, 2016 •

edited

leodido commented Jul 21, 2016 •

edited

motss commented Jul 21, 2016 •

edited

leodido commented Jul 21, 2016 •

edited

leodido Jul 22, 2016 •

edited

motss Jul 22, 2016 •

edited

leodido commented Jul 27, 2016 •

edited

leodido commented Jul 27, 2016 •

edited