1184233 - Tab Tray -> Browser Animations Bugs

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Comment 1

[Stephan Leroux [:sleroux]]

Marking as tracking+ since all dependencies are also tracking+

Comment 2

[Stephan Leroux [:sleroux]]

I created a meta bug to track these issues as I'm finishing up a patch that will resolve the core issues with the animation that manifests itself through these bugs.

Comment 3

[Stephan Leroux [:sleroux]]

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

See PR for commentary. I'd also like to add some UITests for the navigation changes.

Comment 4

[Stephan Leroux [:sleroux]]

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

Hey Aaron,

I was wondering if it would be possible to check a build of this patch for any regressions since it's a pretty large patch. Let me know if you have the bandwidth.

Comment 5

[Aaron Train [:aaronmt]]

(In reply to Stephan Leroux [:sleroux] from comment #4)
> Comment on attachment 8635576 [details] [review]
> PR https://github.com/mozilla/firefox-ios/pull/769
> 
> Hey Aaron,
> 
> I was wondering if it would be possible to check a build of this patch for
> any regressions since it's a pretty large patch. Let me know if you have the
> bandwidth.

Mentioned in IRC but also mentioning here, one issue I'm seeing with https://github.com/mozilla/firefox-ios/pull/769 applied is that with if you close a tab (e.g, the 5th, of 5 tabs open), the remaining 4 bounce down kind of janky (the same applies to landscape).

When that happens, the position of the top row of tabs is set as a lower Y-axis than it initially is when you open a new tab that overflows the overall position causing the top tabs to be partially drawn off-screen.

However, the animation whilst closing a 7th tab brings down 4 to the bottom quicker but the top two tabs come in a tad slower. 

Tested this on my iPhone 6.

Comment 6

[Stephan Leroux [:sleroux]]

Thanks for giving this a test Aaron. I notice this issue on the test flight build 26 so I don't think it's specific to this branch. I'll file a separate bug for it.

Comment 7

[Stephan Leroux [:sleroux]]

Bug for issue found: https://bugzilla.mozilla.org/show_bug.cgi?id=1186604

Comment 8

[Wesley Johnston (:wesj)]

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

Looks good! I still wish the transitioning code was a little more split up into lots of little functions instead of two huge ones, but thats a nit.

Comment 9

[Stephan Leroux [:sleroux]]

Thanks for the review Wes - I'm going to spend some time cleaning up these methods into smaller ones that DRY up the code a bit before merging.

Comment 10

[Stephan Leroux [:sleroux]]

Merged