Closed Bug 1098240 Opened 10 years ago Closed 10 years ago

Use hw layers in tab strip animations

Categories

(Firefox for Android Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 36

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(2 files)

Every little helps.
Attachment #8522153 - Flags: review?(michael.l.comella)
Attachment #8522154 - Flags: review?(michael.l.comella)
Priority: -- → P1
Comment on attachment 8522153 [details] [diff] [review]
Use hw layers in tab strip animations (r=mcomella)

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

::: mobile/android/base/tabs/TabStripView.java
@@ +323,5 @@
> +            setLayerType(View.LAYER_TYPE_HARDWARE);
> +        }
> +
> +        @Override
> +        public void onAnimationEnd(Animator animation) {

nit: You could add a comment here (or in onAnimationCancel) that this is also called when onAnimationCancel is, to save the time of future travellers who are unfamiliar with the framework from having to read the source (e.g. me).
Attachment #8522153 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8522154 [details] [diff] [review]
Remove obsolete comment (r=mcomella)

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

::: mobile/android/base/tabs/TabStripView.java
@@ -123,5 @@
>                  final int childCount = getChildCount();
>                  for (int i = removedPosition - firstPosition; i < childCount; i++) {
>                      final View child = getChildAt(i);
>  
> -                    // TODO: optimize with Valueresolver

Can't seem to find out what a Valueresolver is so why is this out-of-date? What would a Valueresolver do?
Attachment #8522154 - Flags: review?(michael.l.comella) → review+
(In reply to Michael Comella (:mcomella) from comment #4)
> Comment on attachment 8522154 [details] [diff] [review]
> Remove obsolete comment (r=mcomella)
> 
> Review of attachment 8522154 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/tabs/TabStripView.java
> @@ -123,5 @@
> >                  final int childCount = getChildCount();
> >                  for (int i = removedPosition - firstPosition; i < childCount; i++) {
> >                      final View child = getChildAt(i);
> >  
> > -                    // TODO: optimize with Valueresolver
> 
> Can't seem to find out what a Valueresolver is so why is this out-of-date?
> What would a Valueresolver do?

I actually meant PropertyValuesHolder :-) It's only useful when you're animating more than on property in the same object, which is not necessary here.
(In reply to Michael Comella (:mcomella) from comment #3)
> Comment on attachment 8522153 [details] [diff] [review]
> Use hw layers in tab strip animations (r=mcomella)
> 
> Review of attachment 8522153 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/tabs/TabStripView.java
> @@ +323,5 @@
> > +            setLayerType(View.LAYER_TYPE_HARDWARE);
> > +        }
> > +
> > +        @Override
> > +        public void onAnimationEnd(Animator animation) {
> 
> nit: You could add a comment here (or in onAnimationCancel) that this is
> also called when onAnimationCancel is, to save the time of future travellers
> who are unfamiliar with the framework from having to read the source (e.g.
> me).

Ok, done :-)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.