The colors of the icons from the status bar should be in contrast with the status bar background color #12955

iorgamgabriel · 2022-10-17T10:37:52Z

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1795650

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry or does not need one
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650
Fixes #1795650

Amejia481 · 2022-10-17T13:42:20Z

👋🏽
@iorgamgabriel could you add some details about how this PR connect with #12894? I don't have the context around it. Thanks in advance!

Amejia481 · 2022-10-17T13:46:01Z

Should we also add tests to WindowTest?

iorgamgabriel · 2022-10-17T13:47:26Z

@Amejia481 it's not connected to #12894

Amejia481 · 2022-10-17T14:05:52Z

Thanks for the clarification, I think we will need to connect the issue # and update the commit description with it :)

iorgamgabriel · 2022-10-17T15:04:06Z

@Amejia481 I can't write unittests for this, because
fun Window.getWindowInsetsController(): WindowInsetsControllerCompat it is an extension function and I can't mock it.

Amejia481 · 2022-10-17T15:08:35Z

@Amejia481 I can't write unittests for this, because fun Window.getWindowInsetsController(): WindowInsetsControllerCompat it is an extension function and I can't mock it.

We could write tests to verify the expected behaviour, as we could read the value of getWindowInsetsController().isAppearanceLightStatusBars to verify if it has the correct one after calling setStatusBarTheme.

Amejia481 · 2022-10-17T15:16:44Z

Related ticket mozilla-mobile/focus-android#7860 (comment)

Amejia481 · 2022-10-17T15:17:32Z

@mcarare could take a look as you have context around the previous change? :)

components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Window.kt

iorgamgabriel · 2022-10-18T11:32:52Z

getWindowInsetsController().isAppearanceLightStatusBars always returns false does't matter if I send Black or White , I think it is a problem with Robolectric . I made some gui tests for this . @Amejia481

components/support/ktx/src/main/AndroidManifest.xml

components/support/ktx/src/main/java/mozilla/components/support/ktx/MainActivity.kt

...support/ktx/src/androidTest/java/mozilla/components/support/ktx/android/view/WindowKtTest.kt

mergify · 2022-10-18T14:24:48Z

This pull request has conflicts when rebasing. Could you fix it @iorgamgabriel? 🙏

Amejia481

For adding a new UI/Instrumentation tests we need add an entry to the kind.yml file, after it is added you could give it a try by calling bors try :)

iorgamgabriel · 2022-10-19T11:11:10Z

bors try

bors · 2022-10-19T11:29:08Z

try

Build failed:

complete-push

iorgamgabriel · 2022-10-19T11:47:48Z

bors try

bors · 2022-10-19T12:02:25Z

try

Build succeeded:

complete-push

iorgamgabriel · 2022-10-19T13:18:49Z

The tests are failed because they run on API Level 21 and
/**
* Checks if the foreground of the status bar is set to light.
*

* This method always returns false on API < 23.
*
* @return true if the foreground is light
* @see #setAppearanceLightStatusBars(boolean)
*/
public boolean isAppearanceLightStatusBars() {
return mImpl.isAppearanceLightStatusBars();
}

components/support/ktx/src/main/AndroidManifest.xml

Amejia481 · 2022-10-19T13:39:30Z

bors try

bors · 2022-10-19T13:54:23Z

try

Build succeeded:

complete-push

… contrast with the status bar background color.

iorgamgabriel · 2022-10-19T14:29:06Z

bors try

bors · 2022-10-19T14:46:40Z

try

Build succeeded:

complete-push

Amejia481

Great job 🎖️ !

iorgamgabriel requested a review from Amejia481 as a code owner October 17, 2022 10:37

iorgamgabriel requested a review from mcarare October 17, 2022 10:39

iorgamgabriel added the 🕵️‍♀️ needs review PRs that need to be reviewed label Oct 17, 2022

iorgamgabriel force-pushed the 1795650 branch from 94715d7 to 18b57f4 Compare October 17, 2022 10:50

iorgamgabriel force-pushed the 1795650 branch from 18b57f4 to 73c6215 Compare October 17, 2022 14:14

mcarare approved these changes Oct 17, 2022

View reviewed changes

mcarare reviewed Oct 17, 2022

View reviewed changes

components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Window.kt Outdated Show resolved Hide resolved

mcarare reviewed Oct 17, 2022

View reviewed changes

components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Window.kt Outdated Show resolved Hide resolved

iorgamgabriel force-pushed the 1795650 branch 2 times, most recently from c47253d to f29d17f Compare October 18, 2022 08:44

mozilla-mobile deleted a comment from mergify bot Oct 18, 2022

iorgamgabriel force-pushed the 1795650 branch from f29d17f to 4a2e1ff Compare October 18, 2022 11:30

Amejia481 reviewed Oct 18, 2022

View reviewed changes

components/support/ktx/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved

Amejia481 reviewed Oct 18, 2022

View reviewed changes

components/support/ktx/src/main/java/mozilla/components/support/ktx/MainActivity.kt Outdated Show resolved Hide resolved

Amejia481 reviewed Oct 18, 2022

View reviewed changes

...support/ktx/src/androidTest/java/mozilla/components/support/ktx/android/view/WindowKtTest.kt Outdated Show resolved Hide resolved

Amejia481 suggested changes Oct 18, 2022

View reviewed changes

iorgamgabriel force-pushed the 1795650 branch 2 times, most recently from 17341f4 to e4ccfed Compare October 19, 2022 11:05

bors bot pushed a commit that referenced this pull request Oct 19, 2022

Try #12955:

34f854b

iorgamgabriel force-pushed the 1795650 branch from e4ccfed to 572f21c Compare October 19, 2022 11:19

bors bot pushed a commit that referenced this pull request Oct 19, 2022

Try #12955:

65fa092

Amejia481 reviewed Oct 19, 2022

View reviewed changes

components/support/ktx/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved

iorgamgabriel force-pushed the 1795650 branch 2 times, most recently from 45124d8 to ac0ed22 Compare October 19, 2022 13:36

bors bot pushed a commit that referenced this pull request Oct 19, 2022

Try #12955:

34f130e

For #1795650 The colors of the icons from the status bar should be in…

52c7070

… contrast with the status bar background color.

iorgamgabriel force-pushed the 1795650 branch from ac0ed22 to 52c7070 Compare October 19, 2022 14:27

bors bot pushed a commit that referenced this pull request Oct 19, 2022

Try #12955:

bd38ed9

iorgamgabriel requested a review from Amejia481 October 20, 2022 10:08

Amejia481 approved these changes Oct 20, 2022

View reviewed changes

Amejia481 removed the 🕵️‍♀️ needs review PRs that need to be reviewed label Oct 20, 2022

iorgamgabriel added the 🛬 needs landing PRs that are ready to land label Oct 20, 2022

Merge branch 'main' into 1795650

b131a15

mergify bot merged commit eb4f36e into mozilla-mobile:main Oct 20, 2022

Remu-rin mentioned this pull request Nov 19, 2022

[Bug]: Status bar text is unreadable when opening link within another app mozilla-mobile/fenix#24858

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The colors of the icons from the status bar should be in contrast with the status bar background color #12955

The colors of the icons from the status bar should be in contrast with the status bar background color #12955

iorgamgabriel commented Oct 17, 2022 •

edited by github-actions bot

Loading

Amejia481 commented Oct 17, 2022

Amejia481 commented Oct 17, 2022 •

edited

Loading

iorgamgabriel commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

iorgamgabriel commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

iorgamgabriel commented Oct 18, 2022

mergify bot commented Oct 18, 2022

Amejia481 left a comment

iorgamgabriel commented Oct 19, 2022

bors bot commented Oct 19, 2022

iorgamgabriel commented Oct 19, 2022

bors bot commented Oct 19, 2022

iorgamgabriel commented Oct 19, 2022

Amejia481 commented Oct 19, 2022

bors bot commented Oct 19, 2022

iorgamgabriel commented Oct 19, 2022

bors bot commented Oct 19, 2022

Amejia481 left a comment

The colors of the icons from the status bar should be in contrast with the status bar background color #12955

The colors of the icons from the status bar should be in contrast with the status bar background color #12955

Conversation

iorgamgabriel commented Oct 17, 2022 • edited by github-actions bot Loading

Pull Request checklist

After merge

Amejia481 commented Oct 17, 2022

Amejia481 commented Oct 17, 2022 • edited Loading

iorgamgabriel commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

iorgamgabriel commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

Amejia481 commented Oct 17, 2022

iorgamgabriel commented Oct 18, 2022

mergify bot commented Oct 18, 2022

Amejia481 left a comment

Choose a reason for hiding this comment

iorgamgabriel commented Oct 19, 2022

bors bot commented Oct 19, 2022

try

iorgamgabriel commented Oct 19, 2022

bors bot commented Oct 19, 2022

try

iorgamgabriel commented Oct 19, 2022

Amejia481 commented Oct 19, 2022

bors bot commented Oct 19, 2022

try

iorgamgabriel commented Oct 19, 2022

bors bot commented Oct 19, 2022

try

Amejia481 left a comment

Choose a reason for hiding this comment

iorgamgabriel commented Oct 17, 2022 •

edited by github-actions bot

Loading

Amejia481 commented Oct 17, 2022 •

edited

Loading