Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Drop AppTheme flag #11368

Merged
merged 1 commit into from
Aug 1, 2020
Merged

Drop AppTheme flag #11368

merged 1 commit into from
Aug 1, 2020

Conversation

StephaneDelcroix
Copy link
Member

@StephaneDelcroix StephaneDelcroix commented Jul 8, 2020

Description of Change

Drop the AppTheme Flag

Issues Resolved

none

API Changes

None

Platforms Affected

  • Core/XAML (all platforms)
  • iOS
  • Android
  • UWP

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@StephaneDelcroix StephaneDelcroix added a/darkmode t/housekeeping ♻︎ Internal only changes, won't be included in release notes labels Jul 8, 2020
@StephaneDelcroix StephaneDelcroix marked this pull request as draft July 8, 2020 17:58
@StephaneDelcroix StephaneDelcroix removed the t/housekeeping ♻︎ Internal only changes, won't be included in release notes label Jul 8, 2020
@StephaneDelcroix StephaneDelcroix marked this pull request as ready for review July 8, 2020 19:11
@samhouts samhouts self-requested a review July 8, 2020 23:06
@samhouts samhouts self-assigned this Jul 8, 2020
@samhouts samhouts added this to In progress in DarkMode/AppThemes Jul 13, 2020
@samhouts samhouts added the approved Has two approvals, no pending reviews, and no changes requested label Jul 21, 2020
@samhouts
Copy link
Member

@StephaneDelcroix Can you please resolve the conflicts? Thanks!

@samhouts samhouts added this to In Review in vCurrent (4.8.0) Jul 30, 2020
@StephaneDelcroix StephaneDelcroix merged commit 62b5928 into 4.8.0 Aug 1, 2020
vCurrent (4.8.0) automation moved this from In Review to Done Aug 1, 2020
@StephaneDelcroix StephaneDelcroix deleted the appthemeflag branch August 1, 2020 08:10
@samhouts samhouts added this to Done in Sprint 174 Aug 3, 2020
@samhouts samhouts added this to the 4.8.0 milestone Aug 5, 2020
Copy link

@mikebikerider mikebikerider left a comment

Choose a reason for hiding this comment

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

Xamarin.Forms 4.8.0.1269 does not correctly render the iOS status bar area in Dark mode. In the dark mode the status bar area background remains white, while text color switches from black to white (as expected).
Expected behavior for the dark mode is black status bar area background. The issue is easily reproduced with a one page single grid with a 'Welcome to Xamarin Forms!' label. The page content changes correctly via AppThemeBinding applied to the grid background and the Label text color.

That problem did not exist in Xamarin Forms 4.7 builds with AppTheme_Experimental flag.
Android apps are not affected.
IMG_0459
IMG_0458

@vniehues
Copy link

This right here might be needed for it to work properly:

//Dark mode
UIApplication.SharedApplication.SetStatusBarStyle(UIStatusBarStyle.LightContent, true);

//Light mode
UIApplication.SharedApplication.SetStatusBarStyle(UIStatusBarStyle.DarkContent, true);

@mikebikerider
Copy link

mikebikerider commented Aug 29, 2020

Thank you for the suggestion, but it is not a solution.

I slightly modified the test app to better illustrate the issue.

XAML code of a test page producing the screenshot to follow:
Page1

The page has a grid with no background set. One column, three rows. The middle row holds a BoxView and a Label to show the selected Theme and the Xamarin.Forms version.
The issue affects iOS only. I deliberately did not apply UseSafeArea.

IMG_0467

Grid rows 0 and 2 are rendered on the white background while the selected Theme is Dark. The status bar text is not visible because the status bar TextColor is White, as it should be in the Dark mode.

Xamarin Forms 4.7 and prior do render page background according to the current Theme.
Xamarin.Forms 4.8.0.1269 renders page background White regardless of the selected Theme.

A simple but silly looking 'solution' is to put following line into the ContentPage XAML section:
BackgroundColor="{AppThemeBinding Light=White, Dark=Black}"

The below XAML code will produce a correctly rendered page:
Page1 modified

IMG_0468
I don't recall ever applying the background to the whole page. Prior to Xamarin.Forms 4.8 there was no need for this.

Looking closely at the recently updated Xamarin documentation https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/theming/system-theme-changes August 6, 2020:
Xamarin.Forms 4.6.0.967 or greater...
I don't think code snippets offered there will produce iOS dark mode screenshots as shown if rebuilt with Xamarin.Forms 4.8.

I see benefit in setting the Page themed background in XAML. It provides clear WYSISWYG control of the iOS status bar area appearance.
My expectation is if the Page background is not set explicitly in XAML or code the page should load with the background matching the selected theme as it was in 4.6 and 4.7 Xamarin.Forms versions. The build 4.8.0.1269 loads a page on a device (simulator too) in the Dark mode with White background and that screws up the status bar area appearance. I think it should be corrected in a future build.

@stefanhk31
Copy link

So the problem with the background is currently making my app unusable (shot below is of a page with background color set to black on DarkMode, works fine in 4.7):

IMG_0028

This page has text set to white when phone is in Dark Mode, so I believe it is responding to the OS Theme, but somehow not setting the background color properly.

Is there an open issue for this, or do I need to make one?

@mikebikerider
Copy link

Stefan, I can't tell from your screenshot if it is from iPhone or Android phone.
The issue I described affects iOS only. It is easily fixed in XAML by setting ContentPage BackgroundColor :
BackgroundColor="{AppThemeBinding Light=White, Dark=Black}"

Xamarin.Forms 4.7 with the experimental AppTheme flag loads Page with the background color matching the selected theme.
In August I updated two existing apps to Xamarin.Forms 4.8 and didn't even notice until the next day that the system notification area in Dark Mode is rendered white and no text visible because text is correctly rendered in white and not visible on the white background.
Easy to fix in XAML, but is inconsistent.

@stefanhk31
Copy link

Stefan, I can't tell from your screenshot if it is from iPhone or Android phone.
The issue I described affects iOS only. It is easily fixed in XAML by setting ContentPage BackgroundColor :
BackgroundColor="{AppThemeBinding Light=White, Dark=Black}"

Xamarin.Forms 4.7 with the experimental AppTheme flag loads Page with the background color matching the selected theme.
In August I updated two existing apps to Xamarin.Forms 4.8 and didn't even notice until the next day that the system notification area in Dark Mode is rendered white and no text visible because text is correctly rendered in white and not visible on the white background.
Easy to fix in XAML, but is inconsistent.

Sorry, should have clarified. The screenshot above is on iOS. I set the code in XAML exactly as you describe. Background appears as black with the flag on 4.7, but the white screen shows when upgrading to 4.8.

I can give specific more specific code when I'm at my workstation next, but the issue only appeared after I upgraded to 4.8.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/darkmode a/Xaml </> approved Has two approvals, no pending reviews, and no changes requested ControlGallery p/Android p/iOS 🍎
Projects
DarkMode/AppThemes
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

6 participants