mozilla-mobile / prox

[INACTIVE] A search and discovery app for the "here & now". We're experimenting with ideas on mobile that can better surface content from the open web.
https://wiki.mozilla.org/New_Mobile_Experience
Mozilla Public License 2.0
22 stars 12 forks source link

Custom click handling animation blocks fast swiping #169

Open liuche opened 8 years ago

liuche commented 8 years ago

Swiping through the detailed cards on a physical phone, there's a short period after each swipe where subsequent swipes don't work. Not a huge priority, but it does make the app feel a little unresponsive (presumably because the animation handling is eating the swipes).

fluffyemily commented 7 years ago

Polish: Add Animation where required

fluffyemily commented 7 years ago

resolved by #233

kbrosnan commented 7 years ago

This is still an issue. There is a delay when a new card is displayed before you can swipe to the next one.

mcomella commented 7 years ago

I added some print statements and the gesture recognizer isn't called until we're able to swipe again (i.e. we're not short-circuiting).

I wonder if this happens because we're animating – perhaps iOS disables gesture recognizers when animating.

mcomella commented 7 years ago

I wonder if this happens because we're animating – perhaps iOS disables gesture recognizers when animating.

Yep – you can add the .allowUserInteraction option to UIView.animate. However, our code isn't set up to handle it and if you interrupt the animation, the UI breaks (you can't interact, it overlays multiple cards on one another).

mcomella commented 7 years ago

Hack fix ideas:

mcomella commented 7 years ago

If you receive a touch event during an animation, add it to a queue of max size 1. Handle it only after the first animation completes.

This feels crappy - you swipe and then it pauses for a bit as the animation ease completes, then it just randomly swipes. Available on my branch (I only support swiping forward).

But it does make obvious how long the animation's ease out takes to complete – perhaps we can make that feel less drawn out?

sleroux commented 7 years ago

FYI, as part of https://github.com/mozilla-mobile/prox/issues/201 I've been exploring different options for handling how the cards move around and come in. One approach which looks promising is replacing our custom pan gesture/scroll view with a single collection view to:

  1. Allow for more performant recycling when swiping between cards
  2. Allow for smoother transitions between varying vertical offsets of cards
  3. Allow continuous animation handling when swiping between cards since the animation would be baked into the layout attributes.

This is more of a long term fix though since a lot of code would need to be changed for it.

mcomella commented 7 years ago

Spoke with eng: this is complicated and unlikely to happen for the current sprint – resorting.

mcomella commented 7 years ago

To put this somewhere, SO recommends creating a UIScrollView with isPagingEnabled = true. We currently have the scroll view but without paging – perhaps that'd simplify some of the transition handling code we have and fix the issue here.

mcomella commented 7 years ago

Separate from the Prox code, I was able to create a UICollectionView where you can swipe between items in the collection view where the page is smaller than the screen size (allowing you to preview items on the sides). I did not add a ScrollView but I had a plan for it: see my progress in this repository.

That being said, I just realized the reason we went to a custom VC, rather than a CollectionView, was to handle the custom animations between the cards on the side. I have not investigated this heavily but looking at Steph's post above, we may be able to do this by varying the layout attributes of the items, but that notwithstanding, creating a custom UICollectionViewLayout.

Personally, I think the animations are not worth the technical complexity, especially considering how hard it is to change that code.

mcomella commented 7 years ago

This post also lists some methods which could be useful when animating UICollectionView:

prepareLayout prepareForCollectionViewUpdates: finalizeCollectionViewUpdates prepareForAnimatedBoundsChange: finalizeAnimatedBoundsChange shouldInvalidateLayoutForBoundsChange: