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

Android: fix dialog button colors in dark mode #15091

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

webwarrior-ws
Copy link
Contributor

@webwarrior-ws webwarrior-ws commented May 15, 2023

Description

Fix alert dialog button colors in dark mode.
Use ?attr/colorOnSurface for text color in dialogs.
This ensures contrasting color for both dark and light modes.

Use AppCompat version of AlertDialog in picker.
This ensures consistent styling between picker dialog and
alert dialog.

Fixes

Fixes #15088

Screenshots

Now dialogs with default Android theme look like this:

Dark Light
Capture-alertDialog-dark-1 Capture-alertDialog-light-1
Capture-alertDialog-light-2 Capture-alertDialog-dark-2

@ghost ghost added the community ✨ Community Contribution label May 15, 2023
@ghost
Copy link

ghost commented May 15, 2023

Hey there @webwarrior-ws! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 15, 2023
@rmarinho
Copy link
Member

rmarinho commented May 16, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@webwarrior-ws
Copy link
Contributor Author

Added tests for checking brightness of AlertDialog button text to make sure that in dark mode the text is bright (background is dark) and in light mode text is dark (background is light).

parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 17, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 17, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 18, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 18, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 18, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 18, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
public partial class AlertDialogTests: ControlsHandlerTestBase
{

async Task<Color> GetDialogButtonTextColor(int nightMode)
Copy link
Member

Choose a reason for hiding this comment

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

Nice test!

parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 22, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 22, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
parhamsaremi added a commit to parhamsaremi/DG that referenced this pull request May 22, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
knocte pushed a commit to parhamsaremi/DG that referenced this pull request May 23, 2023
Add simple styles.xml to workaround the maui styling bug
in dark theme [1]. Should be removed after merging [2].

[1] dotnet/maui#15088
[2] dotnet/maui#15091
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jul 6, 2023

/azp run

@azure-pipelines

This comment was marked as outdated.

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jul 6, 2023

@webwarrior-ws can you rebase or merge this PR with main please? I'm hoping that'll fix the failing tests. I tried, but it looks like I don't have permission to update your branch.

@jfversluis jfversluis added the s/pr-needs-author-input PR needs an update from the author label Jul 7, 2023
@ghost
Copy link

ghost commented Jul 7, 2023

Hi @webwarrior-ws. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time.

webwarrior-ws and others added 3 commits July 10, 2023 10:31
Fix alert dialog button colors in dark mode.
Use ?attr/colorOnSurface for text color in dialogs.
This ensures contrasting color for both dark and light modes.
Use AppCompat version of AlertDialog in picker.
This ensures consistent styling between picker dialog and
alert dialog.
Add tests for checking brightness of AlertDialog button text.

Co-authored-by: webwarrior <reg@webwarrior.ws>
@webwarrior-ws
Copy link
Contributor Author

@webwarrior-ws can you rebase or merge this PR with main please? I'm hoping that'll fix the failing tests. I tried, but it looks like I don't have permission to update your branch.

Rebased on main.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jfversluis jfversluis enabled auto-merge (squash) July 13, 2023 11:51
@jfversluis jfversluis merged commit cda3eb3 into dotnet:main Jul 13, 2023
@BenEastpoint
Copy link

Hi. any idea when this will be available in a public release? I have the same issue with dialogs

@ISSPRO-Eng
Copy link

Still experiencing this

@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-dialogalert DisplayAlert, dialog community ✨ Community Contribution fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! platform/android 🤖 s/pr-needs-author-input PR needs an update from the author
Projects
None yet