Added checked state part for radio & radio button

[bunesk]

Fixes #931

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
shoelace	✅ Ready (Inspect)	Visit Preview	Oct 5, 2022 at 11:44AM (UTC)

[ErikOnBike]

@claviska
Uhhmm... I might be missing something, but doesn't this change add some "part" instead of a class (or attribute)? I think this is not what was meant by the original issue and this does not solve the issue. (I have the same need to style a checked radio button differently).
I think it would be nice if an attribute (from reflected property) is added, similar to sl-checkbox. Simply changing the checked from a state to a property made it visible in the DOM and allows targeting it from CSS. Or am I missing something?

[bunesk]

@ErikOnBike The thing is that before beta.80 there was a checked attribute for <sl-radio> and <sl-radio-button>. But in beta.80 it got removed and a value attribute was added to <sl-radio-group> instead. So now the value of the radio group decides which radio or radio button is checked:
https://shoelace.style/resources/changelog?id=_200-beta80

If you have also a checked attribute on them would cause two different possibilities to set which one should be checked.
And therefore I added a css part instead which will only be applied to the radio or radio button if it's checked due to the radio group value as suggested by Cory:
#930 (comment)

That means if the next version of shoelace will be released instead of writing

sl-radio-button[checked]::part(button) {
  background-color: red;
}

you must write

sl-radio-button::part(button--checked) {
  background-color: red;
}

and then it should work.

[ErikOnBike]

@Buni48 thanks for the reply. I must have missed the change log comment about 'checked' no longer being present.

Is there a reason why 'checked' on the radio/radio-button is not kept alongside 'value' on the group? Couldn't they co-exist like sl-select has a 'value' and sl-menu-items within it can be 'checked'? If a radio or radio-button is checked (programmatically or by clicking) it could tell its parent/owner (the radio-group) a change is requested. The radio-group could make this happen by de-selecting a previously selected radio or radio-button and selecting the newly requested one (if all validations are okay). This way the group stays in control.
Interested in your thoughts about this.

[bunesk]

I think checked got removed to just have one way to decide which radio is checked to prevent inconsistencies. Because what is your expected behavior if your code looks something like this?:

<sl-radio-group label="Select an option" value="2">
  <sl-radio-button value="1">Option 1</sl-radio-button>
  <sl-radio-button value="2">Option 2</sl-radio-button>
  <sl-radio-button value="3" checked>Option 3</sl-radio-button>
</sl-radio-group>

But you're definitely right that there is an inconsistency with <sl-select> and <sl-menu-item>. I personally would expect that either menu items and radios have a checked attribute or neither of them.
Maybe this inconsistency happened because the menu items are actually the children of the <sl-menu> which is just used by <sl-select> and <sl-menu> has no value property.

I looked what happens if I try this:

<sl-select value="val2">
  <sl-menu-item value="val1">Value 1</sl-menu-item>
  <sl-menu-item value="val2">Value 2</sl-menu-item>
  <sl-menu-item value="val3" checked>Value 3</sl-menu-item>
</sl-select>

What happens is that the value is gonna priorized and the checked attribute will be removed from the third item and set to the second item. Because I usually use the value - which is especially useful for Value Bindings like v-model from Vue or ngModel from Angular - I think this is a good solution. What I also noticed is that changing the checked of menu items will just change them in the menu (so you just can decide where you want your checked icons should be) but the select's value stays untouched.

I understand why this happens but I think maybe adding a value attribute to <sl-menu> and just synchronize it with the <sl-select> would be the thing I would do if I want to make things consistent. Then the checked could be also removed from there.

Your idea that the children (radios etc.) could tell the parent that something changed is also a possible solution but I don't really get the advantage you receive when you can also use a checked attribute. Because for styling a part like button--checked also works. Can you tell an use case where this could be useful? The only use case that comes to my mind is that you have an object you iterate where is a checked property for every item or something like that.
If there is an advantage I would say re-adding checked can make sense but if not I would leave it without to prevent the inconsistencies I've mentioned.

[ErikOnBike]

I agree that inconsistencies can arise and if that is the reason for choosing this approach, that's okay. (I didn't realise at first it was possible to add the --checked part name dynamically, so it does solve the original issue :-).

On a slightly side note:
Inconsistencies are and remain a possibility in declarative HTML of course. If a value is specified on the group which does not exist in the collection of radio or radio-buttons, we have an inconsistency which the code has to handle. Should the group still 'answer' the specified value as its value, although it cannot actually receive this value by user input? Two or more radio or radio-buttons could have (declaratively) received the same value. What does this mean? Which element should be shown as checked?

I use Shoelace mostly programmatically. With the new change I can still programmatically get or set the checked state. It will show visually any change I make. But if I set a radio or radio-button to checked, the already checked radio or radio-button will not become unchecked. This is what I would like to have. I like to see WebComponents as objects with responsibilities, cooperating with other objects. For me it feels logical to give a radio or radio-button the responsibility to maintain its own state and cooperate with its group to make sure the group (parent/owner) stays in a consistent state (ie no two radio or radio-buttons checked at the same time).
I wouldn't mind adding this behaviour to Shoelace components, but I am not sure if this is in line with Shoelace's philosophy or 'direction'.

[claviska]

With the new change I can still programmatically get or set the checked state. It will show visually any change I make. But if I set a radio or radio-button to checked, the already checked radio or radio-button will not become unchecked.

Radios are a special case. The radio group is the form control now, not the radios. This differs from HTML in that the radio group handles the value, and the radio items are managed by the radio group. You can't use a radio without a radio group because of this (similar to not being able to use an <option> outside of a <select>, for example).

The radio group manages a radio item's checked state by setting the checked property. This is intentionally not listed as part of the public API to avoid confusion, especially since that was the way to check them in previous versions.

I'm aware of the inconsistency between this and select. I do plan on revisiting these now that I'll have more time to focus on things like this :)

By the way — radio group was reworked to improve accessibility, as screen readers previously weren't announcing the correct number of options. The controller approach solved that.

[claviska]

For now, a part works so I'm going to merge this since it solves the problem. In the future, I'll be adding more parts for state.

In the ElementInternals future, state will probably change again to use ElementInternals.states, but that's probably a long way off since I don't believe WebKit has started work on this API. Gotta wait for the platform to catch up. Parts are a decent alternative until then.