Closed Bug 843742 Opened 13 years ago Closed 13 years ago

Add an animated transition/slide when flyout sidebars appear and hide

Categories

(Firefox for Metro Graveyard :: Theme, defect)

Product:
Firefox for Metro Graveyard
Component:
Theme
Platform:
All
Windows 8.1
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Firefox 22

People

(Reporter: mbrubeck, Assigned: rsilveira)

References

Details

(Keywords: polish)

Attachments

(1 file, 2 obsolete files)

Using cubic-bezier and duration based on microsoft's winjs animations.
2.59 KB, patch
rsilveira
: review+
Details | Diff | Splinter Review
We originally wanted to animate flyout panels (like "About" and "Options"), but this caused problems because of bug 817641. It should be possible now that bug 817641 is fixed.
Attachment #718108 - Flags: feedback?(mbrubeck)
Comment on attachment 718108 [details] [diff] [review] Added a slide in animation for flyout sidebars. Review of attachment 718108 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/metro/base/content/bindings/flyoutpanel.xml @@ +52,5 @@ > + <body> > + <![CDATA[ > + this.removeAttribute("visible"); > + this.removeEventListener("transitionend", this._onAfterSlideOut); > + DialogUI.popPopup(this); It'd be nice if we could call popPopup() immediately inside hide(), so the user could interact with the rest of the UI without any delay, but the DialogUI isn't built to handle that race condition right now, so putting it here is the safe thing to do for now. @@ +61,5 @@ > <method name="hide"> > <body> > <![CDATA[ > + if (this.isVisible) { > + this.addEventListener("transitionend", this._onAfterSlideOut); Nit: you added some trailing whitespace here. ::: browser/metro/theme/flyoutpanel.css @@ +10,5 @@ > -moz-border-start-style: solid; > visibility: collapse; > position: fixed; > + transition: transform 0.4s ease; > + transform: translateX(100%); You can use percentages for translation transformations?! This is amazing. I learned something new today.
Comment on attachment 718108 [details] [diff] [review] Added a slide in animation for flyout sidebars. Review of attachment 718108 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/metro/base/content/bindings/flyoutpanel.xml @@ +71,5 @@ > > <method name="show"> > <body> > <![CDATA[ > + if (!this.isVisible) { Minor request: Could you change this to if (this.isVisible) return; and make a similar change in the hide() method above? We tend to use early returns like this to make it explicit that nothing further down in the function will execute in this case. (It also has the minor benefit of reducing overall indentation.)
Attachment #718108 - Flags: feedback?(mbrubeck) → feedback+
Attached patch Addressing feedback comments (obsolete) — — Splinter Review
Addressing review comments by mbrubeck and fryn.
Attachment #718108 - Attachment is obsolete: true
Attachment #718504 - Flags: review?(mbrubeck)
Attachment #718504 - Flags: review?(fyan)
Comment on attachment 718504 [details] [diff] [review] Addressing feedback comments Looks good, thanks!
Attachment #718504 - Flags: review?(mbrubeck) → review+
Comment on attachment 718504 [details] [diff] [review] Addressing feedback comments Review of attachment 718504 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/metro/theme/flyoutpanel.css @@ +9,5 @@ > background-color: #ffffff; > -moz-border-start-style: solid; > visibility: collapse; > position: fixed; > + transition: transform 0.4s ease; I'd like to revisit our animations later to see how we compare to the native ones in terms of duration and easing. We don't necessarily have to match the native ones; if the native duration or easing don't work well for us, we can try to pick better ones. Rodrigo mentioned that Microsoft uses exponential functions for easing, which cubic bezier functions can only approximate, but that's okay.
Attachment #718504 - Flags: review?(fyan) → review+
Rodrigo just found that Microsoft uses: duration: 367ms timing-function: cubic-bezier(0.1, 0.9, 0.2, 1) Let's go with that and see how we like it. :)
Status: NEW → ASSIGNED
New animation parameters reviewed through IRC.
Attachment #718632 - Flags: review+
Attachment #718632 - Flags: checkin?(fyan)
Your first patch just landed! Yay! \o/ :D It will be marked RESOLVED FIXED once mozilla-inbound merges in mozilla-central. https://hg.mozilla.org/integration/mozilla-inbound/rev/a0d8482e5532
Target Milestone: --- → Firefox 22
Attachment #718632 - Flags: checkin?(fyan)
Attachment #718504 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a0d8482e5532
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: