Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add missing window flags for styling system bars #32167

Merged
fluttergithubbot merged 10 commits into from
Apr 13, 2022
Merged

Add missing window flags for styling system bars #32167

fluttergithubbot merged 10 commits into from
Apr 13, 2022

Conversation

@camsim99
Copy link
Contributor

@camsim99 camsim99 commented Mar 21, 2022 •
edited
Loading

Addresses the inability to set the icon brightness on system bars on API 30, part of issue #10027.

Using the reproduction included in the issue noted above, I was able to produce the following on a Pixel 2 emulator running API 30 with this change (see before here):

I confirmed these results on Pixel devices running API 28 (with this revert included), 29, and 31.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

@camsim99 camsim99 marked this pull request as ready for review March 22, 2022 00:04
@flutter-dashboard
Copy link

flutter-dashboard bot commented Mar 22, 2022

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@camsim99 camsim99 requested review from Piinks and blasten March 22, 2022 00:05
@camsim99 camsim99 mentioned this pull request Mar 22, 2022
Closed
Piinks
Piinks reviewed Mar 22, 2022
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test that verifies these flag settings independent of the divider color being set?

@camsim99 camsim99 mentioned this pull request Mar 23, 2022
Closed
@camsim99 camsim99 requested review from Piinks March 24, 2022 21:46
@zanderso
Copy link
Member

zanderso commented Mar 31, 2022

From PR review triage: This looks ready for another review pass @blasten @Piinks

blasten
blasten reviewed Apr 5, 2022
View reviewed changes
shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java Outdated
Comment on lines 379 to 382
window.clearFlags(
WindowManager.LayoutParams.FLAG_TRANSLUCENT_STATUS
| WindowManager.LayoutParams.FLAG_TRANSLUCENT_NAVIGATION);

Copy link

@blasten blasten Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These flags are deprecated in API 30

Copy link
Contributor Author

@camsim99 camsim99 Apr 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are needed to fix the problem for API < 30, so I added a comment noting the deprecation and suppressed the warning

blasten
blasten reviewed Apr 5, 2022
View reviewed changes
Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the status bar look like on API 27?

@camsim99
Copy link
Contributor Author

camsim99 commented Apr 8, 2022

What does the status bar look like on API 27?

It looks the same as the pictures above! I also tested this on API 21 to see what happens when none of the operations can be performed (API 23 is needed at least), and it looks the same with and without setting/clearing the flags. I can also move the setting/clearing of flags after the SDK versions are checked for each operation (like here) to rule out side effects, but I was just trying to avoid redundant code.

blasten
blasten approved these changes Apr 13, 2022
View reviewed changes
Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@camsim99 camsim99 added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 13, 2022
@fluttergithubbot fluttergithubbot merged commit 6c62681 into flutter:main Apr 13, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2022
@engine-flutter-autoroll
6c62681 Add missing window flags for styling system bars (flutter/eng…
e4a154e 
…ine#32167)
justinmc pushed a commit to justinmc/engine that referenced this pull request Apr 20, 2022
@camsim99 @justinmc
Add missing window flags for styling system bars (flutter#32167)
16af282
@camsim99 camsim99 mentioned this pull request May 26, 2022
Closed
@bleroux bleroux mentioned this pull request Sep 23, 2022
Merged
@a-wallen a-wallen mentioned this pull request Sep 23, 2022
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Piinks Piinks Awaiting requested review from Piinks

1 more reviewer

@blasten blasten blasten approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs tests platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@camsim99 @zanderso @blasten @Piinks @fluttergithubbot