1151089 - "Send to other devices" has an unexpected transition when clicked

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

It expands rather than sliding in from the bottom, such as when the "Add to Firefox" option is pressed from another application.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Attachment: MozReview Request: bz://1151089/mcomella

/r/6631 - Bug 1151089 - Move slide up animations to onResume. r=liuche

Pull down this commit:

hg pull -r b640e4ccfc429341dbc0b78e04445d56a6417493 https://reviewboard-hg.mozilla.org/gecko/

Comment 2

[Chenxia Liu [:liuche]]

Comment on attachment 8588202 [details]
MozReview Request: bz://1151089/mcomella

https://reviewboard.mozilla.org/r/6629/#review5489

::: mobile/android/base/overlays/ui/ShareDialog.java
(Diff revision 1)
> +        final Animation anim = AnimationUtils.loadAnimation(this, R.anim.overlay_slide_up);

This is fine for this commit, because you're just moving code, but I'm curious why this uses AnimationUtils rather then Animators - is using resource animations faster than creating them dynamically in Java?

Comment 3

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

https://reviewboard.mozilla.org/r/6629/#review5493

> This is fine for this commit, because you're just moving code, but I'm curious why this uses AnimationUtils rather then Animators - is using resource animations faster than creating them dynamically in Java?

Disclaimer: I know nothing about animations.

Background: this was created a while ago when we didn't have much precedent for animation so it's using "legacy" style. ;)

To answer your curiousity, imo, abstracting away animations from code is nice, but I don't know what APIs are available on newer devices (since this uses extremely old APIs).

Comment 4

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/f1da7543f551

Comment 5

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Comment on attachment 8588202 [details]
MozReview Request: bz://1151089/mcomella

Please handle uplifting to 39 (aurora) and I'll handle uplift to 38 as part of uplifting bug 1130203.

Approval Request Comment
[Feature/regressing bug #]: bug 1130203, or so
[User impact if declined]:
  Users will see an improper (and ugly) entry animation when they choose the "Send to other devices" option - i.e. polish.

[Describe test coverage new/current, TreeHerder]: None
[Risks and why]: 
  Low - we change where the animation code is called from so that it happens every time (rather than being placed after an early return).
[String/UUID change made/needed]: None

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f1da7543f551

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Michael, 38 is marked as unaffected, why do you want to uplift it then?

Comment 8

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #7)
> Michael, 38 is marked as unaffected, why do you want to uplift it then?

It's unaffected until I land bug 1130203, which is flagged beta+.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6504e390a559

Comment 10

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

https://hg.mozilla.org/releases/mozilla-beta/rev/358448358c21

Comment 11

[Catalin Suciu [:csuciu]]

Verified fixed Nightly 40.0a1 (2015-04-08) and Aurora 39.0a2 (2015-04-09)
Still needs to be verified on Beta 38.0b4 before closing

Comment 12

[Catalin Suciu [:csuciu]]

Works correctly on Beta 38.0b3 on Sony Xperia Z2 (4.4.4)
Verifying.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Attachment: MozReview Request: Bug 1151089 - Move slide up animations to onResume. r=liuche