1807080 - [Pull To Refresh] Throbber should have dark design in dark mode

Status: VERIFIED FIXED

(Fenix :: General, enhancement)

Description

[Christian Sadilek [:csadilek]]

From github: https://github.com/mozilla-mobile/fenix/issues/16142.

What is the user problem or growth opportunity you want to see solved?

The throbber/spinning animation that appears when pulling to refresh a tab should have an appropriate design when the user has enabled dark mode.

How do you know that this problem exists today? Why is this important?

Currently, the throbber is white in dark mode, which looks strange:

<img src="https://user-images.githubusercontent.com/46764284/96992083-aae66380-1529-11eb-9e42-3041959d9af9.png" width=300 />

Who will benefit from it?

Users with dark mode enabled that use the pull-to-refresh gesture.

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: PullToRefresh.mp4

Investigated this and saw that our implementation was buggy from when it was first implemented in https://github.com/mozilla-mobile/fenix/issues/103.
The current implementation uses a resource id as a color [1].
That resource - R.attr.primaryText supports theming and has different light/dark colors but they are different shades of gray and so if if was to be used correctly (by using swipeRefresh.setColorSchemeResources(primaryTextColor)) in the dark theme a light gray throbber would have very low contrast with it's background and won't be seen.

To fix this we should get new colors to use for the throbber and it's background. We could have different colors for light/dark theme and also for private mode.
The throbber (the rounded arrow) can use an array of colors and would animate between them depending on how much it is being dragged. OR we can use one solid color that would automatically get a fade effect.
The background of it can also be themed to ensure a better contrast between the throbber, the background and the below webpage.

[1] https://github.com/mozilla-mobile/fenix/pull/3020/files#diff-84facb1693d1b185822e99641197460c68ea9bce91c6a438c50111abff9e7da8R352-R353

Comment 2

[Tiffanie Shakespeare [:tif]]

@jane is the color of the refresh indicator in dark mode something crystal could provide? (i don't think she's in bugzilla so i can't tag her direction :D)

Comment 3

[Jane Wolf]

@cwong is her username! I've added her to the ticket here. We will take a look at this this sprint.

Comment 4

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1807080 - Fix PTR dark theme colors (#5455)

Comment 5

[Titouan Thibaud [:titouan]]

Attachment: ptr-dark.mp4

Hey Crystal, we created a patch to fix this bug. Does this dark theme look good for you for the refresh indicator?

(video here)

Comment 6

[BMO Github Automation]

Authored by Titouan Thibaud
https://github.com/mozilla-mobile/firefox-android/commit/c6ed1d76995c7a0946dd8dfd8f089414f075726c
[main] Bug 1807080 - Fix PTR dark theme colors

Comment 7

[Adina Petridean QA [:apetridean]]

Verified as fixed on Nightly 124.0a1 from 02/06 with Motorola Moto G9 plus (Android 11) and OnePlus A3000 (Android 6).

Comment 8

[Adina Petridean QA [:apetridean]]

Attachment: refresh.mp4

Comment 9

[cwong]

@tit(In reply to Titouan Thibaud [:tthibaud] from comment #5)

Created attachment 9378234 [details]
ptr-dark.mp4

Hey Crystal, we created a patch to fix this bug. Does this dark theme look good for you for the refresh indicator?

(video here)

Sorry, I just saw this! It looks good to me! Thank you for making the changes. :)

Comment 10

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1807080 - Fix PTR dark theme colors (backport #5455) (#5541)

Comment 11

[Titouan Thibaud [:titouan]]

Comment on attachment 9379248 [details] [review]
[mozilla-mobile/firefox-android] Bug 1807080 - Fix PTR dark theme colors (backport #5455) (#5541)

Beta/Release Uplift Approval Request

• User impact if declined: The Pull to Refresh indicator keeps it wrong colors in Dark Mode
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It has already been verified by QA and baked in Nightly
• String changes made/needed:
• Is Android affected?: Yes

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9379248 [details] [review]
[mozilla-mobile/firefox-android] Bug 1807080 - Fix PTR dark theme colors (backport #5455) (#5541)

Approved for Mobile 123.0rc1

Comment 13

[BMO Github Automation]

Authored by Titouan Thibaud
https://github.com/mozilla-mobile/firefox-android/commit/a38d60d369543e4e21b1537269eddf51f35798af
[releases_v123] Bug 1807080 - Fix PTR dark theme colors

Comment 14

[Adina Petridean QA [:apetridean]]

Verified as fixed on Firefox RC 123.0.
Device used: Samsung Galaxy S23 Ultra (Android 14) and OnePlus A3000 (Android 6).