1218917 - Pull-to-refresh spinner colors are odd on Android 4.4.2

Status: RESOLVED FIXED

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

Description

[Nick Alexander :nalexander [he/him]]

I am seeing bad colors in the "Material Design" pull-to-refresh spinner on Android 4.4.2.  Possible fallout from Bug 1201206?

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Material.Design.Pull.to.Refresh.Spinner.mp4.webm

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #1)
> Created attachment 8679578 [details]
> Material.Design.Pull.to.Refresh.Spinner.mp4.webm

Hmm, the low-res webm hides some of the grey vs. white mismatching, but the long gap between orange bursts still suggests we're doing something wrong.

Comment 3

[Nick Alexander :nalexander [he/him]]

antlam: can you look at this locally and see if it's what you want?

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

What do we get to define here? I remember our conversation about this before the fallout and I think we were using fennec orange and action orange. 

Can we try that?

Comment 5

[Nick Alexander :nalexander [he/him]]

It looks like these SwipeRefreshLayout's take 4 colors.

At https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/PanelRefreshLayout.java#62 we use 

        setColorSchemeResources(R.color.swipe_refresh_orange, R.color.swipe_refresh_white,
                R.color.swipe_refresh_orange, R.color.swipe_refresh_white);

At https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/RemoteTabsBaseFragment.java#89 we do the same.  So at least we are consistent.

Blame shows that Bug 1183588 removed the old GeckoSwipeRefreshLayout in favour of the built-in one, so I expect vivek will know most here.  vivek, did this change the behaviour?

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Nick Alexander :nalexander from comment #5)
> It looks like these SwipeRefreshLayout's take 4 colors.
> 
> At
> https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/
> PanelRefreshLayout.java#62 we use 
> 
>         setColorSchemeResources(R.color.swipe_refresh_orange,
> R.color.swipe_refresh_white,
>                 R.color.swipe_refresh_orange, R.color.swipe_refresh_white);
> 
> At
> https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/
> RemoteTabsBaseFragment.java#89 we do the same.  So at least we are
> consistent.
> 
> Blame shows that Bug 1183588 removed the old GeckoSwipeRefreshLayout in
> favour of the built-in one, so I expect vivek will know most here.  vivek,
> did this change the behaviour?

Let's try using our fennec orange and action orange as starters then :)

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

^vivek could I get a screenshot when you get a chance? or a test build! heh

Comment 8

[Vivek Balakrishnan[:vivek]]

Attachment: MozReview Request: Bug 1218917 : Fix swipe refresh animation colors r?nalexander

Bug 1218917 : Fix swipe refresh animation colors r?nalexander

Comment 9

[Vivek Balakrishnan[:vivek]]

@Antlam: I have used fennec_ui_orange and action_orange as you mentioned.
The apk file is at https://www.dropbox.com/s/kw4jrxosndocbxi/fennec-45.0a1.en-US.android-arm.apk?dl=0
Please let me if this is okay.

Also, note that we have "swipe_refresh_orange" (#FFFFC26C) and swipe_refresh_white defined. If not needed, I will update the patch to remove them.

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8683648 [details]
MozReview Request: Bug 1218917 : Fix swipe refresh animation colors r?nalexander

https://reviewboard.mozilla.org/r/24403/#review22121

antlam liked the two colors, so bombs away!

Comment 11

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Thanks vivek! I saw the video off Nalexander :)

Comment 12

[Vivek Balakrishnan[:vivek]]

https://hg.mozilla.org/integration/fx-team/rev/2fe63f770d60eb93a919912d98d5afb5761b3366
Bug 1218917 : Fix swipe refresh animation colors r=nalexander

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2fe63f770d60