1366673 - (photon) Tab icon motion for adding a new tab

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Comment 1

[Jing-wei Wu [:jwu]]

Attachment: Bug 1366673 - Part 1: Duplicate tab counter resources for Australis and Photon.

Review commit: https://reviewboard.mozilla.org/r/152530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152530/

Comment 2

[Jing-wei Wu [:jwu]]

Attachment: Bug 1366673 - Part 2: Implement new tab counter for visual and animation refresh.

Review commit: https://reviewboard.mozilla.org/r/152536/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152536/

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8881359 [details]
Bug 1366673 - Part 2: Implement new tab counter for visual and animation refresh.

https://reviewboard.mozilla.org/r/152536/#review157696

::: mobile/android/app/src/photon/java/org/mozilla/gecko/toolbar/TabCounter.java
(Diff revision 1)
> -    }
> -
> -    // Alpha animations in editing mode cause action bar corruption on the
> -    // Nexus 7 (bug 961749). As a workaround, skip these animations in editing
> -    // mode.
> -    void onEnterEditingMode() {

Please make sure this method is not used in other flavor before you remove it.

Comment 4

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8881358 [details]
Bug 1366673 - Part 1: Duplicate tab counter resources for Australis and Photon.

https://reviewboard.mozilla.org/r/152530/#review157700

Comment 5

[Julian Chu [:walkingice]]

Comment on attachment 8881358 [details]
Bug 1366673 - Part 1: Duplicate tab counter resources for Australis and Photon.

https://reviewboard.mozilla.org/r/152530/#review157788

Comment 6

[Julian Chu [:walkingice]]

Comment on attachment 8881359 [details]
Bug 1366673 - Part 2: Implement new tab counter for visual and animation refresh.

https://reviewboard.mozilla.org/r/152536/#review157794

::: mobile/android/app/src/photon/java/org/mozilla/gecko/toolbar/TabCounter.java:76
(Diff revision 1)
> +
> +        // FadeIn in 66ms, with moveDown2 (52~56, 4 frames).
> +        final ObjectAnimator fadeIn = ObjectAnimator.ofFloat(box, "alpha", 0.01f, 1.0f).setDuration(66);
> +
> +        // Move down on y-axis, from -1.0 to 2.7 in 116ms, after moveDown2 (59~66, 7 frames).
> +        final ObjectAnimator moveDown3 = ObjectAnimator.ofFloat(box, "translationY", -1.0f,  2.7f).setDuration(116);

in Android Studio, use 'Ctrl + Alt + L' will find there is one more extra space here

::: mobile/android/app/src/photon/java/org/mozilla/gecko/toolbar/TabCounter.java:175
(Diff revision 1)
> -        // Set In value, trigger animation to Out value
> -        setCurrentText(formatForDisplay(mCount));
> -        setText(formatForDisplay(count));
>  
> -        mCount = count;
> +        // Trigger animation
> +        animationSet.start();

for consistent, maybe `this.animationSet.start()`?

::: mobile/android/app/src/photon/res/drawable/tabs_counter_bar.xml:4
(Diff revision 1)
> +<?xml version="1.0" encoding="utf-8"?>
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +      - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->

it seems redundant space here

::: mobile/android/app/src/photon/res/drawable/tabs_counter_box.xml:4
(Diff revision 1)
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +      - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->

it seems redundant space here

Comment 7

[Jing-wei Wu [:jwu]]

Comment on attachment 8881359 [details]
Bug 1366673 - Part 2: Implement new tab counter for visual and animation refresh.

https://reviewboard.mozilla.org/r/152536/#review157696

> Please make sure this method is not used in other flavor before you remove it.

This method isn't called anymore in Australis, remove it in Photon to keep code clean.

Comment 8

[Jing-wei Wu [:jwu]]

Comment on attachment 8881358 [details]
Bug 1366673 - Part 1: Duplicate tab counter resources for Australis and Photon.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152530/diff/1-2/

Comment 9

[Jing-wei Wu [:jwu]]

Comment on attachment 8881359 [details]
Bug 1366673 - Part 2: Implement new tab counter for visual and animation refresh.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152536/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88f74c51d18c
Part 1: Duplicate tab counter resources for Australis and Photon. r=nechen,walkingice
https://hg.mozilla.org/integration/autoland/rev/3e33da82b722
Part 2: Implement new tab counter for visual and animation refresh. r=nechen,walkingice

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/88f74c51d18c
https://hg.mozilla.org/mozilla-central/rev/3e33da82b722

Comment 12

[Ioana Chiorean]

Pixel Android 7.1 
Nightly 57.0a1 2017-08-08
The animation is pretty fast in my case but it behaves properly 

Please see video: https://youtu.be/yf39v30_ccY