1054778 - Fine-tune rocketbar height/width

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Kevin Grandon :kgrandon]

Right now the rocketbar always collapses into 50% of the fully-expanded width. When there is a back and forward button present the size that it collpases into is smaller than it should be.

Solving this without impacting animation performance is going to be tough, so we may want to see if this is something we can live with for 2.1.

Comment 1

[Kevin Grandon :kgrandon]

Find specs here:

Collapsed spec: https://mozilla.app.box.com/s/y6ubtn3364pvcmoxgis7
Expanded spec: https://mozilla.app.box.com/s/oedme1y7u6m3s6lxvk0a

Comment 2

[Guillaume Marty [:gmarty]]

Just a quick note. If you change the collapsed rocket bar width, please update StatusBar._getMinimizedStatusBarWidth() in apps/system/js/statusbar.js
This function must return the width in pixels available for the status bar when the rocket bar is collapsed.

Comment 3

[Ben Francis [:benfrancis]]

I started to work on this as part of bug 1058096, but it turned into too much of a big change to make as part of that feature so I've posted that commit here instead for reference.

branch: https://github.com/benfrancis/gaia/tree/1054778
commit: https://github.com/benfrancis/gaia/commit/8e52101c13e98cfdc9978b8f9fd4e65bcf55d9b6

We solved this problem in the original implementation of Rocketbar by making sure the expanded width of the Rocketbar input always stretched to 100% before the transition started, so that it always shrunk down to the same size. We also animated the box and the text separately using an embedded transform so that the aspect ratio didn't have to stay exactly the same.

The new implementation uses complicated CSS rules to determine the size of the expanded Rocketbar input so it's always a different size before it starts to shrink down and it always has to maintain the same aspect ratio.

The patch above simplifies this by switching to a flex layout (enabling the removal of lots of CSS) and hiding all the surrounding buttons when the .maximized class is removed so that the Rocketbar input flexes out to 100% width (or actually 90%) before the transition begins so that it's always the same size.

The downside of this approach is that the Rocketbar input immediately jumps in width and the buttons immediately disappear before the collapsing transition starts. This approach will also make it harder to implement the back/forward transitions because I'm not sure whether you can animate flex width.

I'd like to experiment with this further to see if we can polish it.

Comment 4

[Chris Lord [:cwiiis]]

(In reply to Ben Francis [:benfrancis] from comment #3)
> The downside of this approach is that the Rocketbar input immediately jumps
> in width and the buttons immediately disappear before the collapsing
> transition starts. This approach will also make it harder to implement the
> back/forward transitions because I'm not sure whether you can animate flex
> width.
> 
> I'd like to experiment with this further to see if we can polish it.

A possible way to do this animation:

1- Use z-index and put the entry above the other parts of the chrome.
2- Make the entry opaque (if it isn't already) and split it into three components, a left end-cap, the middle entry portion and a right end-cap
3- Lay these entry components out in a position:relative entry container
4- When changing the size of the entry, run translate animations on the end-caps and scale animations on the centre (once this animation has ended, then actually do the sizing)

This should let you get accelerated animations for entry size change, at the expense of more complex layout and code.

I'd advise just letting the entry overlap the controls during the animation to make things easier. If we want to animate away other controls, however, I'd suggest using scale/translate transforms with an appropriately set transform-origin. Alternatively, you can animate a clip rect, but this won't be accelerated (it should still be quite fast for hiding, as it won't trigger extra drawing or reflow, but it does require regeneration of the display-list).

Comment 5

[Kevin Grandon :kgrandon]

I would like to try to visit this in 2.1, but it seems very risky. If we do manage to get a low-risk patch in then let's consider requesting uplift approval. Moving to rocketbar-next for now.

Comment 6

[Ben Francis [:benfrancis]]

Not actively working on this

Comment 7

[Eric Pang [:epang] UX]

Attachment: comparison.jpg

Hey Kevin, here's a comparison of the screens that we were talking about.  I've added an overlap so it's more clear where they differ.

Comment 8

[Eric Pang [:epang] UX]

(In reply to Eric Pang [:epang] from comment #7)
> Created attachment 8490851 [details]
> comparison.jpg
> 
> Hey Kevin, here's a comparison of the screens that we were talking about. 
> I've added an overlap so it's more clear where they differ.

Hey Kevin, have you had a chance to look into this?

Comment 9

[Kevin Grandon :kgrandon]

I will take a look at this.

Comment 10

[Kevin Grandon :kgrandon]

Attachment: Pull Request - Visual Polish

Here is the rocketbar visual polish. I think this is closer to the spec, and I made sure to test as many different cases as I could. This doesn't really match up to the original intent of the bug in regards to shrinking/expanding transitions, but let's use this for the visual polish and can track the collapse/expand separately if needed.

Eric - can you take a look at this on a device and let me know if this matches your expectations? Thanks!

Comment 11

[Eric Pang [:epang] UX]

(In reply to Kevin Grandon :kgrandon from comment #10)
> Created attachment 8494877 [details] [review]
> Pull Request - Visual Polish
> 
> Here is the rocketbar visual polish. I think this is closer to the spec, and
> I made sure to test as many different cases as I could. This doesn't really
> match up to the original intent of the bug in regards to shrinking/expanding
> transitions, but let's use this for the visual polish and can track the
> collapse/expand separately if needed.
> 
> Eric - can you take a look at this on a device and let me know if this
> matches your expectations? Thanks!

Hey Kevin, I agree it's not the original intent, but still a huge win for visual :).
It's really close, just a couple of minor tweaks...
- Input field one pixel up (on flame) to match the RB search field
- Text one pixel to the right (on flame) so it matches up perfectly 

After that I think this will be a smoother transition :)

Thanks for working on this Kevin, it's looking good!

Comment 12

[Kevin Grandon :kgrandon]

(In reply to Eric Pang [:epang] from comment #11)
> It's really close, just a couple of minor tweaks...
> - Input field one pixel up (on flame) to match the RB search field
> - Text one pixel to the right (on flame) so it matches up perfectly 

Thanks Eric. These two issues are now addressed in the pull request. Could you give it another spin?

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8494877 [details] [review]
Pull Request - Visual Polish

Eli - since you introduced CSS variables into the system app, I think you would make a good reviewer for this. Would you mind taking a look? Thanks!

Comment 14

[:Eli Perelman]

Comment on attachment 8494877 [details] [review]
Pull Request - Visual Polish

I think this looks great!

Comment 15

[Kevin Grandon :kgrandon]

In master: https://github.com/mozilla-b2g/gaia/commit/3090e390bef48ed425ec355e18451acb266be716

Comment 16

[Kevin Grandon :kgrandon]

Comment on attachment 8494877 [details] [review]
Pull Request - Visual Polish

This visual polish makes the rocketbar experience a lot less jarring, and adds some much needed polish.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Missing polish during feature implementation.
[User impact] if declined: Poor visual experience.
[Testing completed]: CSS changes, so only manual testing, and ui-review.
[Risk to taking this patch] (and alternatives if risky): Should be very low risk as it's just Visual changes.
[String changes made]: No.

Comment 17

[Eric Pang [:epang] UX]

Comment on attachment 8494877 [details] [review]
Pull Request - Visual Polish

This looks great, thanks for the quick fix Kevin! R+

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.1 uplift.

Comment 19

[Kevin Grandon :kgrandon]

Caused my LTR work in master, but the uplift was pretty simple: https://github.com/mozilla-b2g/gaia/commit/718427f14ac3a560cd687b43db343ca773843850

Comment 20

[SandKing]

This issue has been verified successfully on Flame2.1&2.2
Verify video:"verify_1054778.mp4".

Flame 2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880

Flame2.2 bulid:
Gaia-Rev        bd4dcc8c4582e2368b47b0e62506d3031fb2fc09
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/f1f48ccb2d4e
Build-ID        20141208160206
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141208.191634
FW-Date         Mon Dec  8 19:16:45 EST 2014
Bootloader      L1TC00011880

Comment 21

[SandKing]

Attachment: Verified video: verify_1054778.mp4