RadioButtons doesn't respect BorderThickness or BorderBrush property

[mdmozibur]

Setting these two properties has no effect on the Control.
I am not sure if it's bug or just intentionally designed in this way.

[Felix-Dev]

@Muziburrahman I agree that these properties should have an effect:

<RadioButtons Header="Options header"
              BorderBrush="Gray" BorderThickness="2">
    <x:String>Option 1</x:String>
    <x:String>Option 2</x:String>
    <x:String>Option 3</x:String>
<RadioButtons>

should result in:

Alongside the BorderThickness and BorderBrush APIs we should at least also support the Background, Padding and CornerRadius APIs, so that we can do something like this:

<RadioButtons Header="Options header"
              BorderBrush="Gray" BorderThickness="2"
              Background="DodgerBlue"
              Padding="5"
              CornerRadius="4">
    <x:String>Option 1</x:String>
    <x:String>Option 2</x:String>
    <x:String>Option 3</x:String>
<RadioButtons>

If we decide to respect these APIs, we should also consider adding matching theme resources like

• RadioButtonsBorderBrush
• RadioButtonsBorderThickness
• RadioButtonsBackground

@StephenLPeters @YuliKl FYI.

Open questions

Should we respect the CornerRadius API? My current train of thought is that if we decide to support APIs like BorderBrush then it also makes sense to support the CornerRadius API since rounded borders is a common UI scenario. I assume developers would expect support for this here (without having to manually create a Border control). The current default value for the control corner radius is 2 so if we add CornerRadius support here, that would automatically round the corners of the RadioButtons control like this:

<RadioButtons Background="DodgerBlue">
    <x:String>Option 1</x:String>
    <x:String>Option 2</x:String>
    <x:String>Option 3</x:String>
<RadioButtons>

And with a header:

That looks fine to me.

[mdmozibur]

Also it should have a default Padding. Without padding it looks ugly.

[StephenLPeters]

I don't think we need the resources right? we add resources for customization points on template parts which are not exposed on the root control. These customization points will be settable on RadioButtons' without a resource.

[mdmozibur]

I don't think we need the resources right? we add resources for customization points on template parts which are not exposed on the root control. These customization points will be settable on RadioButtons' without a resource.

I agree. I think it would be just creating those properties and then TemplateBinding them with the root StackPanel.

[Felix-Dev]

@StephenLPeters I suggested creating resources because that would match the current resource system. The Button control, for example, has ButtonBackground and ButtonForeground theme resources, even though we can already customize the foreground and background of a button with the Background and Foreground APIs (same goes for the TextBox and other controls).

[StephenLPeters]

ButtonBackground is used as the default value for the Background property (via a setter). This is useful for two reasons; First it allows us to pick a non-standard default value, second it allows an app to change all of their button's backgrounds by setting the ButtonBackground resource. In this case I don't think that we should be choosing a non-standard default value, however the second reason I think is still applicable. @MikeHillberg do you have thoughts?

[ranjeshj]

@kikisaints to weigh in for light weight styling here.

[kikisaints]

I agree we should have lightweights styling resources for this control. Even if the default is transparent.

Looking at generic.xaml for RadioButton (non-plural) we have the following definitions:

...
<StaticResource x:Key="RadioButtonBackground" ResourceKey="SystemControlTransparentBrush" />
<StaticResource x:Key="RadioButtonBackgroundPointerOver" ResourceKey="SystemControlTransparentBrush" />
<StaticResource x:Key="RadioButtonBackgroundPressed" ResourceKey="SystemControlTransparentBrush" />
<StaticResource x:Key="RadioButtonBackgroundDisabled" ResourceKey="SystemControlTransparentBrush" />
<StaticResource x:Key="RadioButtonBorderBrush" ResourceKey="SystemControlTransparentBrush" />
<StaticResource x:Key="RadioButtonBorderBrushPointerOver" ResourceKey="SystemControlTransparentBrush" />
...

They may all be set to transparent because they are not being used in our current design system definition, however I don't think that negates the need for them to be available.

We should have something similar for RadioButtons as well.

[Felix-Dev]

@StephenLPeters Would you be fine with me tackling both this issue and #3350 in a single PR so we won't get merge conflicts to hold us back? (I.e. I would add new API tests to cover both issues which would create merge conflicts if we have two separate PRs.)

Or would you favor a clear separation here even if that means merge conflicts will block PR merge for a short time?

Edit: I think I will just create two PRs as usual.

[StephenLPeters]

Either way is fine with me.

[Felix-Dev]

@kikisaints @StephenLPeters I am thinking about not adding a Pressed visual state for the RadioButtons control (so no *Pressed theme resources like RadioButtonsBackgroundPressed). The RadioButtons control has no in-built press action, compared to its individual RadioButton items (or other controls like Buttons/ListView items). As such, it would look weird to me if clicking on the RadioButtons UI surface would change the background of the RadioButtons control as that would suggest to me a click action is about to be performed.

Customers could still use the RadioButtons.PointerPressed event to wire up custom click logic but that shouldn't be a common scenario. And if they absolutely need to change the background of the RadioButtons control when clicked, then they can always manually set the background color of the control in their PointerPressed event handler.

So, for now I plan to add theme resources for the normal visual state (default background, border brush), the PointerOver visual state and the Disabled visual state. If, for example, the mouse pointer enters the RadioButtons control, then we enter the PointerOver visual state. If the customer then clicks somewhere on the RadioButtons UI surface (i.e. a RadioButton) we remain in the PointerOver visual state (as there is no Pressed visual state):

<muxc:RadioButtons
               BorderBrush="LightGray"
               BorderThickness="2"
               Padding="5"
               CornerRadius="4"
                   Header="RadioButtons control">
    <muxc:RadioButtons.Resources>
        <SolidColorBrush x:Key="RadioButtonsBackgroundPointerOver" Color="DodgerBlue"/>
        <SolidColorBrush x:Key="RadioButtonsBorderBrushPointerOver" Color="Gray"/>
    </muxc:RadioButtons.Resources>
    <x:String>Option 1</x:String>
    <x:String>Option 2</x:String>    
</muxc:RadioButtons>

If we aren't already in the PointerOver visual state when clicking on the RadioButtons control then we will remain in the current visual state.

Your thoughts?

[Felix-Dev]

@kikisaints Friendly ping.

[michael-hawker]

Yeah, it's a bit odd that the control is just using a StackPanel with no properties set as it's container. All the Border type properties should at least by using TemplateBinding to the parent controls to allow this pass-thru to occur (and CornerRadius, etc...). Surprised this wasn't caught during the initial PR. Was just going to raise this same issue, as it prevents re-styling to mimic #2310 easily.

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.