[WinRT/UWP] Apply BackgroundColor to Stepper buttons #581

Merged
merged 4 commits into from Jan 3, 2017

Conversation

Projects
None yet
5 participants
@pauldipietro
Member

pauldipietro commented Nov 30, 2016

Description of Change

When presently used in WinRT/UWP, the Stepper can potentially have the background color run the width of the screen due to the color being applied on the panel behind its buttons opposed to the buttons themselves (which are transparent to let the color show through). By adding a DependencyProperty on the StepperControl which works on the buttons themselves, this can be made to look more as expected.

A present side effect is that changing the color while one of the buttons is not enabled will make the button appear as that color instead of gray until it returns to that state (e.g. the stepper is at its minimum value of 0 and the color changes; the minus stepper will appear as that color until it is incremented, then decremented) but it's probably an edge case at best.

Bugs Fixed

https://bugzilla.xamarin.com/show_bug.cgi?id=48236

API Changes

(In the StepperControl implementations in WinRT/UWP)

  • public static readonly DependencyProperty ButtonBackgroundColorProperty
  • public Color ButtonBackgroundColor

Behavioral Changes

This will potentially change how the Stepper looks in a user's app, but should be more as expected without the color running over. There's the edge case of changing the color, which I didn't particularly look that far into. If a user really wanted to use the BackgroundColor as before, they could presumably override UpdateBackgroundColor on a custom renderer and use the BackgroundColor as before.

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

@hartez hartez self-assigned this Dec 27, 2016

+// Apply the default category of "Issues" to all of the tests in this assembly
+// We use this as a catch-all for tests which haven't been individually categorized
+#if UITEST
+[assembly: NUnit.Framework.Category("Issues")]

This comment has been minimized.

@hartez

hartez Dec 27, 2016

Member

This attribute only needs to be declared once for the assembly; you can remove if from this file.

@hartez

hartez Dec 27, 2016

Member

This attribute only needs to be declared once for the assembly; you can remove if from this file.

+ {
+ protected override void Init()
+ {
+ var stepper = new Stepper

This comment has been minimized.

@hartez

hartez Dec 27, 2016

Member

Could we add some explanatory text for whoever is verifying this test? There's nothing to tell the person running it whether it failed.

@hartez

hartez Dec 27, 2016

Member

Could we add some explanatory text for whoever is verifying this test? There's nothing to tell the person running it whether it failed.

@@ -18,6 +18,8 @@ public sealed class StepperControl : Control
public static readonly DependencyProperty IncrementProperty = DependencyProperty.Register("Increment", typeof(double), typeof(StepperControl),
new PropertyMetadata(default(double), OnIncrementChanged));
+ public static readonly DependencyProperty ButtonBackgroundColorProperty = DependencyProperty.Register("ButtonBackgroundColor", typeof(Color), typeof(StepperControl), new PropertyMetadata(default(Color), OnButtonBackgroundColorChanged));

This comment has been minimized.

@hartez

hartez Dec 27, 2016

Member

We should probably use nameof(ButtonBackgroundColor).

@hartez

hartez Dec 27, 2016

Member

We should probably use nameof(ButtonBackgroundColor).

@@ -15,6 +15,8 @@ public sealed partial class StepperControl : UserControl
public static readonly DependencyProperty IncrementProperty = DependencyProperty.Register("Increment", typeof(double), typeof(StepperControl),
new PropertyMetadata(default(double), OnIncrementChanged));
+ public static readonly DependencyProperty ButtonBackgroundColorProperty = DependencyProperty.Register("ButtonBackgroundColor", typeof(Color), typeof(StepperControl), new PropertyMetadata(default(Color), OnButtonBackgroundColorChanged));

This comment has been minimized.

@hartez

hartez Dec 27, 2016

Member

Another nameof opportunity.

@hartez

hartez Dec 27, 2016

Member

Another nameof opportunity.

@hartez

This comment has been minimized.

Show comment
Hide comment
@hartez

hartez Dec 29, 2016

Member

The test failing on iOS 10 (CellsGalleryImageUrlCell) has nothing to do with these changes; that failure can be ignored.

Member

hartez commented Dec 29, 2016

The test failing on iOS 10 (CellsGalleryImageUrlCell) has nothing to do with these changes; that failure can be ignored.

+ // Refer to https://bugzilla.xamarin.com/show_bug.cgi?id=48236 for this issue. WinRT/UWP had
+ // an issue where the background color would be used on the area containing the buttons,
+ // potentially causing the color to run the width of the screen. Only the buttons should have
+ // a background color.

This comment has been minimized.

@hartez

hartez Dec 30, 2016

Member

The comments are good, but I meant on-screen text for anyone manually running the test. So they have some idea of what constitutes success and failure (e.g., the on-screen text for 36014 ).

@hartez

hartez Dec 30, 2016

Member

The comments are good, but I meant on-screen text for anyone manually running the test. So they have some idea of what constitutes success and failure (e.g., the on-screen text for 36014 ).

@hartez

hartez approved these changes Dec 30, 2016

@rmarinho rmarinho merged commit 6fc18e0 into master Jan 3, 2017

5 of 6 checks passed

iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests failed: 18…
Details
Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 351, i…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3691, ignored: 10
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests passed: 344…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 346…
Details

rookiejava added a commit to rookiejava/Xamarin.Forms that referenced this pull request Jan 9, 2017

[WinRT/UWP] Apply BackgroundColor to Stepper buttons (#581)
* [WinRT/UWP] Apply BackgroundColor to Stepper buttons

* Add explanatory text; use nameof

* Move explanatory text to a label

rmarinho added a commit that referenced this pull request Feb 2, 2017

Add pressed and released events to Button (#446)
* Add pressed and released events to Button

* Update ButtonRenderer.cs

* Apply safely casting to android button renderer

* Use safety casting for Android buttin renderer

* [Windows] Fix modal pages being laid out below soft buttons (#395)

* Add sample HanselForms and TwitterDemo to ControlGallery (#651)

* [Controls] Add Hanselforms sample

* Remove extra twitter sample

* [Controls]Add TwitterDemo sample

* [Controls] Fix build

* Slider should show user-set value on initial load (#378)

* [UWP] Use toolbar foreground color on primary items (#640)

* Avoid duplicating code in OmPlatform (#591)

* [iOS] Entry should not pass a newline to the next responder (#397)

* UITextField should not return so that the next field does not get passed a newline

* Added code sample

* [XamlC] import members on x:Static and factories (#642)

* [Xaml] support short Properties for PropertyCondition (#645)

* Xamlc compile data triggers (#648)

* [Xaml] DataTrigger and PropertyCondition no longer use a ServiceProvider

* [XamlC] avoid generating ServiceProvider for unused ProvideValue

* fix tests

* Fix comment typo

* [UWP] Fix TextBox style for foreground focus color (#618)

* Adding image to use for CellsGalleryImageUrlCellList UI test

* Update ImageCellListPage to use an image we control;
Update CellsGalleryImageUrlCellList test to wait longer than 1s for images
to load if necessary

* fix nre when changing content in datepickerselected (#494)

* Make CellsGalleryImageUrlCellList test finish early if possible

* [iOS] Change keyboard type while keyboard is visible (#443)

* Change keyboard while changing text

* add sample code

* [Android] Fix NavigationPage dispose crash when it parents a MasterDetailPage (#577)

* fix navigation page dispose crash

* changes after review

* [XamlC] detect duplicate x:Name at compile time (#655)

* [XamlC] detect duplicate x:Name at compile time

* invoking methods with the right arguments produces better results

* Make UWP toolbar display rules consistent with other platforms (#638)

* Allow subscriber to be collected if MessagingCenter is the only reference to it (#617)

* Repro

* Make messaging center callbacks weak references

* Preserve attribute

* Fix test method name

* Watch for collection of actual delegate target instead of wrapper delegate

* Preserve the original platform instance when changing main page

* Better tests for lambda situations

* Update tests, make callback target a weakreference if it's the subscriber

* Ensure old Platform MessagingCenter subs are gone before creating new Platform

* [iOS] Prevent multiple ListView cells from being swiped simultaneously (#578)

* disable multiple cell swipe

* add sample code

* refactored

* convert to weakreference

* remove null setting

* change weakreference setting place

* remove if

* revert isopen changes

* add instructions

* [WinRT/UWP] Apply BackgroundColor to Stepper buttons (#581)

* [WinRT/UWP] Apply BackgroundColor to Stepper buttons

* Add explanatory text; use nameof

* Move explanatory text to a label

* Return group instead of internal class (#461)

* [iOS/Android] Move Map camera to correct region on layout change (#548)

* Move to region on layout change

* remove visibility check

* [iOS] Platform specifics for controlling Picker SelectedIndex change behavior (#540)

* picker selected index could change when picker view is dismissed

* use enum

* [iOS] Ignore intermittent failing test on XTC (#666)

* [UITest] Update to UITest 2.0.5 (#665)

* Rebase the current branch onto upstream latest

@rmarinho rmarinho deleted the fix-bugzilla48236 branch Jun 22, 2017

@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