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

[Windows] Fix Transparent Default Button #468

Merged
merged 1 commit into from Nov 8, 2016

Conversation

Projects
None yet
4 participants
@jimmgarrido
Collaborator

jimmgarrido commented Oct 18, 2016

Description of Change

This fixes two issues introduced in #31:

  1. The default background for a button is transparent
  2. Background color set in a static resource style is ignored

When a FormsButton is first loaded, BackgroundColor will be null if the user did not set a color. In this case we will use the native control's Background instead as it will either be the default theme color or one from a style.

Bugs Fixed

API Changes

None

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@StephaneDelcroix

I'm having a hard time trying to understand what you're trying to do here. could you please explain ?

@jimmgarrido

This comment has been minimized.

Show comment
Hide comment
@jimmgarrido

jimmgarrido Oct 20, 2016

Collaborator

Currently if a Button is added without a non-default background color set, then UpdateBackground() inside the ButtonRenderer is not called. This means that FormsButton.BackgroundColor is null when setting the ContentPresenter background. And since the native button's Background is also set to transparent, this results in only the button text being visible when it is rendered.

Given this scenario, this fix will copy the native Button's background color before it's set to transparent to FormsButton.BackgroundColor so that we can use it for the ContentPresenter.Background. This is done because at that point the native Button's background will either be the the system theme default or a color defined in a static resource style that targets a Button.

Collaborator

jimmgarrido commented Oct 20, 2016

Currently if a Button is added without a non-default background color set, then UpdateBackground() inside the ButtonRenderer is not called. This means that FormsButton.BackgroundColor is null when setting the ContentPresenter background. And since the native button's Background is also set to transparent, this results in only the button text being visible when it is rendered.

Given this scenario, this fix will copy the native Button's background color before it's set to transparent to FormsButton.BackgroundColor so that we can use it for the ContentPresenter.Background. This is done because at that point the native Button's background will either be the the system theme default or a color defined in a static resource style that targets a Button.

@samhouts

Tested this with the Button gallery to see if the buttons actually look like buttons now, and tested case 39853 to ensure #31 was not regressed. All looks well. Thanks, @jimmgarrido!

@StephaneDelcroix StephaneDelcroix merged commit 8aa29ec into xamarin:master Nov 8, 2016

@jimmgarrido jimmgarrido deleted the jimmgarrido:uwp_button_background branch Dec 13, 2016

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment