Fennec's Doorhanger animations should follow desktop

RESOLVED FIXED in Firefox 23

Status

()

Firefox for Android
Theme and Visual Design
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: sriram, Assigned: wesj)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 23
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: ui-hackathon)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Desktop slides down the doorhangers and fades in them from the "anchor". Fennec zooms in the doorhangers from the "anchor". We should probably have similar approach for animations across all the products.
Whiteboard: ui-hackathon
(Assignee)

Updated

5 years ago
Assignee: nobody → wjohnston
(Assignee)

Comment 1

5 years ago
Looks like desktop does this 20px slide in thing: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/popup.css#32
(Assignee)

Comment 2

5 years ago
Created attachment 741512 [details] [diff] [review]
Patch v1

Fix
Attachment #741512 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 741512 [details] [diff] [review]
Patch v1

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

Maybe provide a test build for Ian's feedback before pushing? Just avoid another round of tweaks later.

::: mobile/android/base/resources/values/styles.xml
@@ +427,5 @@
>          <item name="android:padding">10dp</item>
>          <item name="android:textAppearance">@style/TextAppearance</item>
>      </style>
>  
> +    <style name="Popup" />

This seems unnecessary, no?
Attachment #741512 - Flags: review?(lucasr.at.mozilla)
Attachment #741512 - Flags: review?(ibarlow)
Attachment #741512 - Flags: review+
This looks awesome Wes. Can we try it with a transition that's about 50% faster?
(In reply to Lucas Rocha (:lucasr) from comment #3)
> Comment on attachment 741512 [details] [diff] [review]
> Patch v1
> 
> ::: mobile/android/base/resources/values/styles.xml
> @@ +427,5 @@
> >          <item name="android:padding">10dp</item>
> >          <item name="android:textAppearance">@style/TextAppearance</item>
> >      </style>
> >  
> > +    <style name="Popup" />
> 
> This seems unnecessary, no?

Popup.Animation means the style is inheriting from Popup (see http://developer.android.com/guide/topics/ui/themes.html#Inheritance), so the Popup style must be declared.
(In reply to Brian Nicholson (:bnicholson) from comment #6)
> (In reply to Lucas Rocha (:lucasr) from comment #3)
> > Comment on attachment 741512 [details] [diff] [review]
> > Patch v1
> > 
> > ::: mobile/android/base/resources/values/styles.xml
> > @@ +427,5 @@
> > >          <item name="android:padding">10dp</item>
> > >          <item name="android:textAppearance">@style/TextAppearance</item>
> > >      </style>
> > >  
> > > +    <style name="Popup" />
> > 
> > This seems unnecessary, no?
> 
> Popup.Animation means the style is inheriting from Popup (see
> http://developer.android.com/guide/topics/ui/themes.html#Inheritance), so
> the Popup style must be declared.

Yeah, I know. We discussed this on IRC. I meant more that creating an empty style as a "namespace" is not really a good practice. I suggested simply adding a PopupAnimation style.

Updated

5 years ago
Attachment #741512 - Flags: review?(ibarlow)
https://hg.mozilla.org/mozilla-central/rev/b6187222b5ac
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Blocks: 1011535
You need to log in before you can comment on or make changes to this bug.