Closed Bug 813288 Opened 7 years ago Closed 7 years ago

Fennec's Doorhanger animations should follow desktop

Categories

(Firefox for Android :: Theme and Visual Design, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 23

People

(Reporter: sriram, Assigned: wesj)

References

(Blocks 1 open bug)

Details

(Whiteboard: ui-hackathon)

Attachments

(1 file)

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: nobody → wjohnston
Attached patch Patch v1Splinter Review
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.
Attachment #741512 - Flags: review?(ibarlow)
https://hg.mozilla.org/mozilla-central/rev/b6187222b5ac
Status: NEW → RESOLVED
Closed: 7 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.