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

Add Radiobutton.RadioColor #10411

Closed
wants to merge 7 commits into from
Closed

Add Radiobutton.RadioColor #10411

wants to merge 7 commits into from

Conversation

jfversluis
Copy link
Member

@jfversluis jfversluis commented Apr 22, 2020

Description of Change

Adds the ability to style the "radio" (open for better naming) independently.

Issues Resolved

API Changes

Added:

  • Color RadioButton.RadioColorProperty // BindableProperty
  • Color RadioButton.RadioColor

Platforms Affected

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

Behavioral/Visual Changes

When upgrading nothing happens, only when you start using the new RadioColor this will show up.

For UWP I have tried to match it as close to the native experience, however with the vanilla radiobutton you would have some dimming effects etc. when you hover over the radio button. That would mean dynamically determine colors or implement some manual dimming effect.

Before/After Screenshots

Before

image

image

image

After

image

image

image

Testing Procedure

Go to the gallery app and find the radio button page. On the "groups" page there are now a red, yellow and blue radio button. There is also a separate page that lets you set a random color and shows what that does on the disabled state of it.

PR Checklist

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

@jfversluis jfversluis added t/bug 🐛 API-change Heads-up to reviewers that this PR may contain an API change blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. a/style a/radiobutton 🔘 labels Apr 22, 2020
@jfversluis jfversluis added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Apr 22, 2020
@jfversluis
Copy link
Member Author

While screenshotting the before and after scenarios I noticed that I am changing the default Android behavior. Added DNM while I figure out how to keep that instated

@jfversluis
Copy link
Member Author

OK, Android is fixed. Good to go afaic!

@jfversluis jfversluis removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Apr 22, 2020
@@ -39,6 +42,12 @@ public string GroupName
set { SetValue(GroupNameProperty, value); }
}

public Color RadioColor
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just call it Color?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m fine with anything really. I just figured it would make it more clear with RadioColor to what it refers.
And it’s a bit more consistent with other properties like BackgroundColor or TextColor etc.

"Radio" might not be the best, but I guess it's called RadioButton for a reason. So that thing is called the Radio, right? :P

Copy link
Contributor

Choose a reason for hiding this comment

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

I expected Color as well...

Copy link
Contributor

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

Looks good overall

  1. how hard would it be to add OutlineColor?
  2. On UWP I'd go through the VSM on the template and remove anything that affects the color so that pressed and hover colors stay whatever you set from forms
  3. If possible I'd add some code to check if the user has specified a VSM. If they have then don't set a disabled alpha color (there's an existing bug with this on Button though so this could probably wait for another PR)

@@ -39,6 +42,12 @@ public string GroupName
set { SetValue(GroupNameProperty, value); }
}

public Color RadioColor
Copy link
Contributor

Choose a reason for hiding this comment

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

I expected Color as well...

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Apr 30, 2020
@davidortinau davidortinau added this to the 4.7.0 milestone May 24, 2020
@samhouts samhouts removed the p/iOS ?? label Jun 20, 2020
@samhouts samhouts changed the base branch from 4.6.0 to 4.7.0 June 25, 2020 22:23
@samhouts samhouts added t/enhancement ➕ and removed t/bug 🐛 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. labels Jun 25, 2020
@samhouts samhouts changed the base branch from 4.7.0 to master June 25, 2020 22:37
@samhouts samhouts added blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. t/bug 🐛 labels Jun 25, 2020
@samhouts samhouts changed the base branch from master to 4.8.0 June 26, 2020 00:05
@samhouts samhouts removed the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Jul 13, 2020
@RhomGit
Copy link

RhomGit commented Jul 17, 2020

Just curious if this also addresses AppThemeBinding? I am having unusual results switching between Light/Dark.

            <Style TargetType="RadioButton">
                <Setter Property="TextColor" Value="{AppThemeBinding Dark=White, Light=Black}" />
            </Style>

image

@jfversluis
Copy link
Member Author

@RhomGit I would open a new issue for that. In fact while I'm here, the probability of this getting merged is pretty low. Closing this for now.

@jfversluis jfversluis closed this Jul 17, 2020
@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Aug 13, 2020
@viktorszekeress
Copy link

What happened with this change? It's not in XF 4.8, nor 5.0 pre3

@jfversluis
Copy link
Member Author

@viktorszekeress the radiobutton implementation has been completely changed so this didn't make any sense anymore. At least not code-wise.

@PureWeen PureWeen deleted the radiobutton-radiocolor branch January 7, 2021 22:36
@MelissaW990
Copy link

How do we change the colour of the circle in a radio button now then?

@tompi
Copy link

tompi commented Jan 18, 2021

Ouch, spent a few hours on trying to set color of a radiobutton in xf 5... Guess its back to inputkit :(

@developer9969
Copy link

@PureWeen is it possible to set the color of radioButton in XF5 stable? Do we have to use renderers?

@jmielczarkowski
Copy link

@developer9969 @tompi hello, you can use as temporary workaround setting own style on App.xaml resource dictionary with keys:

  <!--  Circle color for Radio Button.  -->
  <SolidColorBrush x:Key="RadioButtonCheckMarkThemeColor" Color="#FFFFFF" />
  <SolidColorBrush x:Key="RadioButtonThemeColor" Color="#FFFFFF" />

hope that helps ;)

@InDieTasten
Copy link

I've now created a custom control (with RadioColor property) and custom renderers, which extend the default renderers in a way, that this pull request is adding it directly inside the default renderer and it works perfectly fine. Light-Dark-Mode are not my concern at the moment.

I would wish for this to be added to XF4 and 5, so we don't have to do this silly act of doing everything custom.
Having some bugs in edge cases is better than not supporting this altogether. It's embarrassing having to tell management we can't change the color of the radio input, within 4 hours of development time, because we have to factor in these shenanigans.

@nchrisr
Copy link

nchrisr commented Oct 18, 2021

I've now created a custom control (with RadioColor property) and custom renderers, which extend the default renderers in a way, that this pull request is adding it directly inside the default renderer and it works perfectly fine. Light-Dark-Mode are not my concern at the moment.

I would wish for this to be added to XF4 and 5, so we don't have to do this silly act of doing everything custom. Having some bugs in edge cases is better than not supporting this altogether. It's embarrassing having to tell management we can't change the color of the radio input, within 4 hours of development time, because we have to factor in these shenanigans.

@InDieTasten Would you mind sharing some code on your implementation of this? It would be very helpful for me. Thanks

@InDieTasten
Copy link

@InDieTasten Would you mind sharing some code on your implementation of this? It would be very helpful for me. Thanks

I cannot. We ultimately removed our Radio buttons, as we had continued issues with them. They have been replaced with a switch control.

I recommend using a separate controls library for radio buttons, if you have more than two options in a group.

@dpuckett
Copy link

dpuckett commented Apr 6, 2022

How do we change the colour of the circle in a radio button now then?

Have to ask this question as well. Whilst Maui is still not supported for production, this question is still VERY relevant!
Please can a solution be provided as this is ridiculous.

@jfversluis @PureWeen

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/radiobutton 🔘 a/style API-change Heads-up to reviewers that this PR may contain an API change ControlGallery DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. p/Android p/iOS 🍎 p/UWP retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. t/bug 🐛 t/enhancement ➕
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Unable to style RadioButton icon color