Various improvements related to dismissing items #1
Conversation
| @@ -186,8 +209,63 @@ | |||
| startPosition: currentYTranslate, | |||
| curve: 'ease-in-out', | |||
| }); | |||
| this.startAnimations(400); | |||
There was a problem hiding this comment.
I don't see this delay in android apps. Which one were you looking at?
There was a problem hiding this comment.
I think we can pull in this change once we address this issue.
There was a problem hiding this comment.
I was testing GMail. It doesn't allow dismissing any items while it is animating the collapse, thus there is a slight delay before the collapse animation starts. Whether the delay is 200ms or 400ms is a bit hard to say.
Without the delay you could only dismiss one item at the time.
Could you test with GMail on Android? The spam folder is very useful for this purpose :-)
There was a problem hiding this comment.
I see. This seems super weird to me. If you happen to get in under the delay then it doesn't do the collapse animation, but that's really difficult to do. I'd rather we not replicate this specific behavior. It's just gonna make the app feel unresponsive.
There was a problem hiding this comment.
I think I have a way to work around it. Let me try that
|
OK @ojanvafai I fixed the last comment and rebased the patch now that Erik made some changes. |
|
@ojanvafai fixed now. |
|
Rebased and removed some commits which are no longer needed |
| if (item.animationController && !item.animationController.isAnimationComplete()) | ||
| return; | ||
| dismissInProgressCount: 0, | ||
| itemsToAnimate: [], |
There was a problem hiding this comment.
This needs to be on the instance and not on the prototype... or does polymer magically make it so?
There was a problem hiding this comment.
I looked at this and the documentation says:
Important: Be careful when initializing properties that are objects or arrays. Due to the nature of prototype, you may run into unexpected “shared state” across instances of the same element. If you’re initializing an array or object, do it in ready() rather than directly on the prototype.
So I will move the array assignments to the ready method.
|
This is really nice. Mostly nits at this point. |
|
@arv thanks! I uploaded a new version with the fixed for the issues you pointed out. |
| var currentXTranslate = this.style.WebkitTransform ? this.style.WebkitTransform.split('(')[1].split(',')[0] : 0; | ||
| this.style.WebkitTransform = 'translate3d(' + currentXTranslate + ',' + yPosition + 'px,0)'; | ||
| function getTransformValue(item) { | ||
| return item.transformValue_ || 0; |
|
LGTM @ojanvafai Do you usually squash things together or should we just merge and don't care about extra commits? |
When dismissing we now delete the item from the fakeData. For doing so an id is added to all items. After that we refresh all the items. Also, make sure items visually after the dismissed item are moved up The visual order is not necessarily the same as the DOM order as the children can be split into two groups with different y transform.
Like the dismissItem, 'animateHeightOnDismiss' now also removes the dismissed item from the fake dataset. The code has been separated out in the dismissItemAnimateHeight method similar to the dismissItem one.
The app-dismissable-list now fires a 'dismiss-items' event, which is hooked up to the data provider. When the data provider is changed it sends out a custom event 'data-changed' which is ScrollingEngine listens to. For this to work a proper FakeDataProvider object has been created.
This is based on the behavior on Android - Collapse only happens when all ongoing dismissals have been either cancelled or completed. - Do not allow dismissing items while running collapse animation For the above to work the DismissController has been enhanced with the ability to deactive it (ie not allow dismissing items during animation) as well as gotten a callback when a dismiss was cancelled, and when the user started a potential dismissal.
|
OK, I fixed the off by one indentation. If you want, I can squash them all |
|
We don't have a usually. Either way is fine with me.
|
Various improvements related to dismissing items
Multiple patches improving the the dismissing of items:
Feedback welcome. I can easily merge and split patches if needed.