1163695 - Improve tab tray animations to match UX mocks

Status: RESOLVED FIXED

(Firefox for iOS :: Theme & Visual Design, defect)

Description

[Darrin Henein [:darrin]]

This transition is still choppy and with deltas from the designed animation. It is central to the experience so want to track remaining refinements here. I have a WIP branch that can be used here: https://github.com/darrinhenein/firefox-ios/tree/TabTrayUIPolish

Comment 1

[Wesley Johnston (:wesj)]

Steven is going to grab this branch and get it landed for you.

Comment 2

[Stephan Leroux [:sleroux]]

Attachment: PR https://github.com/mozilla/firefox-ios/pull/475

Pushed up some of the work I've been doing to port in Darrin's animation changes and TabTrayController refactor. I've refactored a lot of it and wouldn't mind some feedback what I'm doing so far.

Comment 3

[Wesley Johnston (:wesj)]

Comment on attachment 8607885 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/475

Put a couple questions in the PR. I like the lazy vars a lot (and this code needed a good cleanup, so thanks. I will no longer have to be embarrassed when I see CustomCell).

I'm curious about the why we need to snapshot the collectionview and hopeful we can actually make the urlBar/BrowserToolbar animation a little less heavy (but any fix should help the animation a lot).

Comment 4

[Stephan Leroux [:sleroux]]

Hey Wes, thanks a lot for checking that PR out. I really liked the lazy pattern. Glad you found it nice as well. I've been doing some work on this branch today and made the following realizations:

1. Having the URLBarView/Toolbar as part of the TabContentView is causing the animation to be really sluggish. I ended up removing them and currently trying to get the transforms working from the BVC.
2. Moved the frame layout code in the TabContentView back to autolayout since that wasn't where the performance issues were.
3. Trying to tackle getting the CALayer transforms working with the BVC :( I might put a pause on this branch and visit the URLBarView. I'm seeing issues transforming this during the animation because it has the status bar built into it (related to https://bugzilla.mozilla.org/show_bug.cgi?id=1164897)

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8607885 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/475

Looks good to me!

Comment 6

[Stephan Leroux [:sleroux]]

Comment on attachment 8607885 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/475

Going to mark this patch as obsolete and break apart the refactoring from the animation changes.

Comment 7

[Stephan Leroux [:sleroux]]

Attachment: PR https://github.com/mozilla/firefox-ios/pull/516

Ported over the animation code from darrin's branch

Comment 8

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8612354 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/516

Code LGTM. I played with this on an iPad Mini and iPhone 6. Did not notice anything odd. Will leave visual check to Darrin.

Comment 9

[Darrin Henein [:darrin]]

Comment on attachment 8612354 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/516

This feels GREAT. Much improved over what we have currently. Ship it!

Comment 10

[Stephan Leroux [:sleroux]]

Merged.