Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moves windowLayoutInDisplayCutoutMode & windowLightNavigationBar usage to API 27 #15138

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

oguzkocer
Copy link
Contributor

Fixes #15077 in combination with #15085 & #15135.

windowLayoutInDisplayCutoutMode & windowLightNavigationBar properties are only available in API >= 27, so this PR adds the styles that use this property to values-v27/styles.xml and removes the unavailable property from the default values/styles.xml file.

To test:

This PR doesn't have any impact on the current UI, but there are a couple things the reviewer can check if they want to be extra sure.

  1. In current develop go into values/styles.xml and find the usages for these properties. Android Studio will complain about them not being available before API 27 and offer to add the values to values-v27 which results in this diff. (it won't remove the properties from values/styles.xml, that needs to be done manually)
  2. For the more paranoid, which I kind of was because I wasn't sure how the resources were merged (if at all), one can open two emulators one for API < 27 and one for API >= 27, run the app, select dark mode, go to about page and verify they look correct. Then, to see the different values in play, one can change WordPress.NoActionBar.DarkNavbar's android:navigationBarColor in these two different places to different colors. This color will show up at the bottom navigation bar in the about page. Note that, if the values-v27 color is completely removed, but the values one is assigned to something like orange, the emulator with API >= 27 will still show it as black. This tells me that styles with the same name for different apis are not merged. They instead get assigned directly.

The second test is really unnecessary, but I just wanted share what I did to understand the behavior.

Regression Notes

  1. Potential unintended areas of impact
    Incorrect styling for the changed WordPress.Stories.Immersive and WordPress.NoActionBar.DarkNavbar styles

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manually tested the UI as described in the testing steps

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@oguzkocer oguzkocer added this to the 18.0 milestone Aug 5, 2021
@oguzkocer oguzkocer requested a review from a team August 5, 2021 04:46
@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@oguzkocer oguzkocer mentioned this pull request Aug 5, 2021
3 tasks
@peril-wordpress-mobile
Copy link

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

👍

@oguzkocer oguzkocer merged commit 9088afb into develop Aug 5, 2021
@oguzkocer oguzkocer deleted the fix-lint-errors-for-requires-api-27 branch August 5, 2021 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New lint errors in Gradle 7.1.1 & Android Gradle Plugin 4.2.2
2 participants