feat(ui-library): radio group - use slots instead of options array

[bar-tay]

closes #507

[faselbaum]

Hey @bar-tay 👋

I looked into the ACs as well as the changes and i found that we're not really using slots the way they are intended here.

We are still not using the blr-radio component inside the radio group but instead render custom input elements. We want to get rid of the custom radio rendering inside blr-radio-group.

There are two main problems that arise from this that we should IMO tackle here:

Problem 1 - How to render blr-radio components?

I see two ways we can handle this. Both of which require some research and experimentation but in both cases we should expect users of blr-radio-group to pass blr-radio elements as children. This needs to be documented.

A) Create near perfect clones of the slotted elements

This would require cloning and observing any changes to the source elements via MutationObservers and probably modify source elements, defining trackable get/set accessors for all properties (if that's even possible) to proxy any changes to our clones.

• Pro: We don't manipulate element instances or values provided by consumers which is considered bad practice (in most cases)
• Con: Hard to implement if even possible. Even harder to implement cleanly.

B) Use the slotted elements directly

We could just display the slotted elements.

• Pro: Easy to implement
• Con: Working with shared instances can lead to all sorts of synchronisation issues when manipulating and reading variables from multiple actors. This Needs extra attention on state management and signalling to ensure that our controlling component (blr-radio-group) doesn't interfere with consumer code in unexpected ways.

Problem 2 - Loss of native input type="radio" behavior

The input type="radio" elements inside our blr-radio component will lose some of their internal behaviour as they are isolated from one another due to shadow-dom encapsulation.

One of those behaviours we lose, but which is needed for a radio group, is changing the state of other radio's of the same group to "off" (defined by the name="" attribute) when the checked state changes to "on".

In short: Click one radio in the group. Other radios will automatically turn off.

We need to re-implement this behaviour with JS inside of blr-radio-group.

For this to work we need to ensure that blr-radio correctly updates the value of the checked property when the user clicks on the element. This is currently not the case.

Further considerations (Not necessarily part of this PR)

• When considering Problem 1 - Solution B: How do we know when a JS program changes the checked state of one of the blr-radio elements? We don't fire any events in this case. In this case our blr-radio-group would get out of sync and possibly have two or more radios selected simultaneously

[angsherpa456]

Hi @bar-tay JFYI when you are working further on this. The second commit 6a5c68a. achieves the desired behaviour based on the click event (The wrapper of the slot in radio-group component listens to the event bubbled by the slotted element[radio-button]). However, the following part still needs to be solved.

"When considering Problem 1 - Solution B: How do we know when a JS program changes the checked state of one of the blr-radio elements? We don't fire any events in this case. In this case our blr-radio-group would get out of sync and possibly have two or more radios selected simultaneously"

[thrbnhrtmnn]

I have found one small thing in Storybook which seems not to appear on current develop.

When hasHint = true and I also switch hasError to true, it seems like the margin between radios and caption group increases. Further inspections make me think, that the reason is that the radio slots increase in height when hasError = true.

[thrbnhrtmnn]

Hey @angsherpa456 ,
thanks the last issue I reported was fixed with the new commits. Unfortunately I found new issues after looking deeper into the component, please let me know which of these should be separate tickets and which you still want to fix within this PR:

• For the Radios the pressed state is implemented wrong if you you click on the radio control, it is implemented correct when you click on the label
• For the Radios the disabled and readonly state are not displayed correctly
• The Legend has a 2 px padding left and right, which I do not see in design
• The dark theme is not implemented for the radios

[thrbnhrtmnn]

Hey @angsherpa456 , I checked the component and all points except the one we discussed will be a separate task (first in the list) have been solved.

[angsherpa456]

Hi @thrbnhrtmnn here is the issue as separate task for the first in the list #1138