Closed Bug 1177524 Opened 5 years ago Closed 5 years ago

[Control Center] Improve subview animations

Categories

(Firefox :: General, defect, P1)

defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 42
Iteration:
42.1 - Jul 13
Tracking Status
firefox41 --- verified
firefox42 --- verified

People

(Reporter: ttaubert, Assigned: ttaubert)

References

Details

(Whiteboard: [fxprivacy] [campaign])

Attachments

(6 files, 2 obsolete files)

The animation looks a little bumpy, let's improve it.
Flags: qe-verify+
Flags: firefox-backlog+
Attached image Original state
Two problems here:

1) When closing the subview the content immediately disappears before it moves out to the right.

2) When closing the subview the "More Information" button jumps to the top and then slides down again.
This fixes the jumping button.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8626315 - Flags: review?(gijskruitbosch+bugs)
Iteration: --- → 41.3 - Jun 29
Attached image State with patch 1
Wanted to apply that same patch to the main menu, but esp. the devtools button seems to handle subviews itself and then they still disappear.
Attachment #8626323 - Flags: review?(gijskruitbosch+bugs)
Attached image State with patch 2
Comment on attachment 8626315 [details] [diff] [review]
0001-Bug-1177524-Apply-max-height-0-after-transitioning-t.patch

Review of attachment 8626315 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/customizableui/content/panelUI.xml
@@ +13,5 @@
>        <stylesheet src="chrome://browser/content/customizableui/panelUI.css"/>
>      </resources>
>      <content>
>        <xul:box anonid="viewContainer" class="panel-viewcontainer" xbl:inherits="panelopen,viewtype,transitioning">
> +        <xul:stack anonid="viewStack" xbl:inherits="viewtype,transitioning" viewtype="main" class="panel-viewstack">

I wonder if this will help simplifying some of the CSS.
Attachment #8626315 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8626323 [details] [diff] [review]
0002-Bug-1177524-Don-t-hide-the-subview-until-after-the-t.patch

Review of attachment 8626323 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/shared/controlcenter/panel.inc.css
@@ +29,5 @@
>  .panel-mainview[panelid=identity-popup] {
>    min-width: 30em;
>  }
>  
> +#identity-popup-securityView:not([mainview]):not([current]) {

I think we should probably use this everywhere? Can you update the onSubviewHidden thing to fire after the transition finishes (assuming that doesn't cause "interesting" races with clicking to open a view right after closing one...).
Attachment #8626323 - Flags: review?(gijskruitbosch+bugs)
Ok that took me a while to come up with but it works for the control center, the history subview, and the devtools subview now.

We're basically replacing display:none with visibility:collapse to preserve that non-visible elements do not affect the layout, but we can transition from visiblity:visible for the currently shown subview to visibility:collapse after the transition is done.

Moved the clearSubview() calls for history and devtools subviews to onPanelShowing() so that their contents stick around for the transition.
Attachment #8626323 - Attachment is obsolete: true
Attachment #8627512 - Flags: review?(gijskruitbosch+bugs)
Can do with one rule less actually.
Attachment #8627512 - Attachment is obsolete: true
Attachment #8627512 - Flags: review?(gijskruitbosch+bugs)
Attachment #8627513 - Flags: review?(gijskruitbosch+bugs)
Iteration: 41.3 - Jun 29 → 42.1 - Jul 13
Rank: 1
Comment on attachment 8627513 [details] [diff] [review]
0002-Bug-1177524-Don-t-hide-the-subview-until-after-the-t.patch, v3

Review of attachment 8627513 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM for now. Part of me wonders if/how this works with keyboard a11y because now those items linger around. We could probably kill post-transition but that's more work, let's punt on that for now and we can revisit if necessary.

::: browser/base/content/browser.css
@@ +113,5 @@
>    transition: transform 150ms;
>  }
>  
>  panelview:not([mainview]):not([current]) {
> +  transition: visibility 0s linear 150ms;

Pretty please make a CSS variable for the 150ms so that we reuse it between the transform on which this is waiting, and here. :-)
Attachment #8627513 - Flags: review?(gijskruitbosch+bugs) → review+
QA Contact: mwobensmith
Approval Request Comment
[Feature/regressing bug #]: bug 1170759
[User impact if declined]: Subpar and jumpy animations when showing and hiding subviews in the control center. 
[Describe test coverage new/current, TreeHerder]: No test coverage for animations.
[Risks and why]: Low risk, touches only the animation itself.
[String/UUID change made/needed]: None.
Attachment #8629205 - Flags: review+
Attachment #8629205 - Flags: approval-mozilla-aurora?
Verified fixed in Fx42.0a1, 2015-07-06.
Status: RESOLVED → VERIFIED
Comment on attachment 8629205 [details] [diff] [review]
Merge of both for Aurora uplift

Approving for uplift as it has been in m-c for a while.
Attachment #8629205 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [fxprivacy] → [fxprivacy] [campaign]
Verified fixed FF 41b3 Win 7, Ubuntu 14.04, OS X 10.10.
You need to log in before you can comment on or make changes to this bug.