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] Correctly notify CheckBox Color changes #7084

Merged
merged 5 commits into from
Jul 20, 2022
Merged

[Android] Correctly notify CheckBox Color changes #7084

merged 5 commits into from
Jul 20, 2022

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

Correctly notify CheckBox Color changes.

<Style TargetType="CheckBox">
    <Setter Property="Color" Value="{AppThemeBinding Light=Yellow, Dark=Orange}" />
    <Setter Property="BackgroundColor" Value="{AppThemeBinding Light=White, Dark=Black}" />
</Style>

Light
light-issue-1

Dark
dark-issue-1

Issues Fixed

Fixes #6253

@Eilon Eilon added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 12, 2022
@rmarinho rmarinho requested review from hartez and PureWeen May 12, 2022 17:09
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Can we add a test for this like this: https://github.com/dotnet/maui/blob/main/src/Controls/tests/DeviceTests/Elements/Page/PageTests.cs#L67-L84

Preferably we need a way to see if a property changes that a set of handler events fire, but this is technically better as we check for colors.

@jsuarezruiz
Copy link
Contributor Author

Can we add a test for this like this: https://github.com/dotnet/maui/blob/main/src/Controls/tests/DeviceTests/Elements/Page/PageTests.cs#L67-L84

Preferably we need a way to see if a property changes that a set of handler events fire, but this is technically better as we check for colors.

Updated adding device tests.

Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

The test logic should be cross-platform, so should we move the test into the shared code so it runs on all platforms?

@jsuarezruiz
Copy link
Contributor Author

The test logic should be cross-platform, so should we move the test into the shared code so it runs on all platforms?

Yeah, moved to shared code to allow run it on all the platforms. Waiting for the build.

@rmarinho
Copy link
Member

Can you rebase @jsuarezruiz ?

@PureWeen PureWeen changed the base branch from main to net6.0 July 19, 2022 22:19
@PureWeen PureWeen requested a review from mattleibow July 19, 2022 22:35
@PureWeen PureWeen merged commit 3787b6e into net6.0 Jul 20, 2022
@PureWeen PureWeen deleted the fix-6253 branch July 20, 2022 15:23
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts added fixed-in-6.0.486 Look for this fix in 6.0.486 SR4! fixed-in-7.0.0-rc.1.6683 labels Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android] CheckBox styles not changing with AppThemeBinding
6 participants