Alternative style for CheckboxRadioSwitch (radio type)

[julien-nc]

This was inspired by nextcloud/server#26691

A new boolean prop buttonVariant toggles between the classic style and this one. It can be used with all types.

If the CheckboxRadioSwitch elements are displayed next to each other vertically, their borders are "merged".

radio3.mp4

[jancborchardt]

Nice! :) Some design feedback:

• The last entry without icon should probably still be indented so the text is left-aligned with the others.
• Currently each individual item has a border-radius on all corners – but actually it is a group which should only have border-radius on the 4 outside corners. So only on the top left and top right of the first entry and the bottom left and bottom right of the last entry.
• Since we are increasing border-radius across the board, it’s probably best to go with var(--border-radius-large) for this

[julien-nc]

@jancborchardt Thanks for the feedback

The last entry without icon should probably still be indented so the text is left-aligned with the others.

The content is a slot. Its styling is done outside this component 😁.

Since we are increasing border-radius across the board, it’s probably best to go with var(--border-radius-large) for this

Yep.

Currently each individual item has a border-radius on all corners...

Completely agree... but: I did it like that initially and then I realized the radio elements are not necessarily next to each other so we need to handle both cases, IMO:

• when they are next to each other, only round corners of first and last elements
• when they don't "touch" each other, round each corners of each element

The main problem here is that it's impossible to select a predecessor in CSS (only a successor with +). Apparently it will come with CSS4 but whatever.

An alternative is to add props to set border-radius manually. This might make more sense because it's weird to try to make a component aware of what's around it in order to define its own style.

The video in the first comment has been updated.

[skjnldsv]

This should be automatically calculated.
Grouped-by-name radio can easily be targeted with first and last :)

[julien-nc]

Alright, now using :first-of-type and :last-of-type.

We still need to get the information about whether they are direct successors or not (grouped or not). Still using a prop for that.

I also added the ability to group vertically (video is up-to-date in the first comment).

[raimund-schluessler]

We still need to get the information about whether they are direct successors or not (grouped or not). Still using a prop for that.

I don't think that's necessary (or even helpful). Grouped checkboxes need to be direct siblings with no checkboxes as siblings that are not grouped. So they need to be wrapped in a common container. Otherwise the markup will fail, without or with the grouped prop.

See this for example:

<template>
<div>
	<div>
	First toggle with three options
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="r" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read</CheckboxRadioSwitch>
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read+write</CheckboxRadioSwitch>
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read+write</CheckboxRadioSwitch>
	</div>
	<div>
	Second toggle with two options
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="r" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read</CheckboxRadioSwitch>
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read+write</CheckboxRadioSwitch>
		Other toggle not grouped
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true">Default permission read+write</CheckboxRadioSwitch>
		<br>
		sharingPermission: {{ sharingPermission }}
	</div>
</div>
</template>
<script>
export default {
	data() {
		return {
			sharingPermission: 'r',
		}
	}
}
</script>

Which gives:

Notice that the corners of the second last checkbox are missing, since there is another one not belonging to the group afterwards. For the three grouped checkboxes wrapped in a container, it works fine.

So imho we only need a prop to indicate horizontal/vertical grouping (would be nice to figure that out by CSS as well, but I don't know how) and leave the rest to the proper markup (give a common container to the grouped boxes).

[raimund-schluessler]

We still need to get the information about whether they are direct successors or not (grouped or not). Still using a prop for that.

I don't think that's necessary (or even helpful). Grouped checkboxes need to be direct siblings with no checkboxes as siblings that are not grouped. So they need to be wrapped in a common container. Otherwise the markup will fail, without or with the grouped prop.

See this for example:

<template>
<div>
	<div>
	First toggle with three options
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="r" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read</CheckboxRadioSwitch>
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read+write</CheckboxRadioSwitch>
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read+write</CheckboxRadioSwitch>
	</div>
	<div>
	Second toggle with two options
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="r" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read</CheckboxRadioSwitch>
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true" :button-variant-v-grouped="true">Default permission read+write</CheckboxRadioSwitch>
		Other toggle not grouped
		<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true">Default permission read+write</CheckboxRadioSwitch>
		<br>
		sharingPermission: {{ sharingPermission }}
	</div>
</div>
</template>
<script>
export default {
	data() {
		return {
			sharingPermission: 'r',
		}
	}
}
</script>

Which gives:

Notice that the corners of the second last checkbox are missing, since there is another one not belonging to the group afterwards. For the three grouped checkboxes wrapped in a container, it works fine.

So imho we only need a prop to indicate horizontal/vertical grouping (would be nice to figure that out by CSS as well, but I don't know how) and leave the rest to the proper markup (give a common container to the grouped boxes).

[jancborchardt]

Looks great design-wise now! :)

@eneiluj regarding the border-radius, it could also be done by adding a parent/container and using first- and last-child on that.

[julien-nc]

So they need to be wrapped in a common container. Otherwise the markup will fail, without or with the grouped prop.

@raimund-schluessler What I meant is that from within the CheckboxRadioSwitch component, we can't know if all of the related CheckboxRadioSwitch are direct siblings (or if there are elements in between) so we need a prop for that. Of course they need to have a common direct parent but we need to know the intention of the developer as we can't guess it with CSS.

@jancborchardt If you mean adding one container around all the CheckboxRadioSwitch of a group, this is not possible as this component is only one element.
If you mean a container around each CheckboxRadioSwitch I don't see what difference it makes 😁.
Also, last-child is not enough as you can have other type of children in the same container so it would not select what we want for sure.

Thank you both for the feedback 💙 .

So we can now choose if we display them grouped with the buttonVariantGrouped prop. This has an effect on the first/last (selected with :first-of-type and :last-of-type) element's border radius and it merges the borders between elements.
When not grouped, all elements have rounded borders.

outdated

[raimund-schluessler]

So they need to be wrapped in a common container. Otherwise the markup will fail, without or with the grouped prop.

@raimund-schluessler What I meant is that from within the CheckboxRadioSwitch component, we can't know if all of the related CheckboxRadioSwitch are direct siblings (or if there are elements in between) so we need a prop for that. Of course they need to have a common direct parent but we need to know the intention of the developer as we can't guess it with CSS.

Yes, that's true. I just wanted to point out, that, if Checkboxes are grouped, there must not be any other Checkboxes in the container that don't belong to this group. Otherwise the :*-of-type selector won't lead to the desired result.

[dartcafe]

Why not adding a new component like https://github.com/nextcloud/polls/blob/master/src/js/components/Base/RadioGroupDiv.vue?

[marcoambrosini]

Hi, I think that the style of this component departs a bit from our design language, most notably due to the use of borders, which is something we've been trying to move away for some time. cc @jancborchardt
I know it might be too late for change but I'll leave an Idea of a similarly behaving component that we're using in our UI and whose style could probably fit here.

This also happens to be somewhat consistent with how they do radios in material design

[raimund-schluessler]

The code looks good and works as intended.

Regarding the design, I think @marcoambrosini proposal looks nice as well, but that's up to @nextcloud/designers to decide and can be improved later on.

Also, I would be in favor of the grouped checkboxes container component @dartcafe proposed, because

1. grouped checkboxes obligatorily need a parent element containing only the checkboxes belonging to the group (otherwise the style fails) and a provided container component would make this more clear
2. then the container would only need a single prop indicating whether the boxes are grouped vertically or horizontally

So something like this could be the result:

<CheckboxRadioSwitchGroup direction="vertical/horizontal">
	<CheckboxRadioSwitch :checked.sync="sharingPermission" value="r" name="sharing_permission_radio" type="radio" :button-variant="true">Default permission read</CheckboxRadioSwitch>
	<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rw" name="sharing_permission_radio" type="radio" :button-variant="true">Default permission read+write</CheckboxRadioSwitch>
	<CheckboxRadioSwitch :checked.sync="sharingPermission" value="rwx" name="sharing_permission_radio" type="radio" :button-variant="true">Default permission read+write+execute</CheckboxRadioSwitch>
</CheckboxRadioSwitchGroup>

[julien-nc]

@marcoambrosini I agree with @raimund-schluessler, in the end that's the decision of @nextcloud/designers. I completely understand if this is not merged because it does not go the same direction as most of the other components 😁 .

@raimund-schluessler @dartcafe About the CheckboxRadioSwitchGroup component, I think it would be nice indeed. The button-variant prop could be moved in CheckboxRadioSwitchGroup because all CheckboxRadioSwitch in a group will most likely have the same aspect.

[dartcafe]

I can start a PR and move our implementation to this repository. I guess there is some more work to do - besides the documentaion - because it currently fits into our use cases.

But I am also fine with it, if someone starts over.

[marcoambrosini]

I'm totally onboard with creating a new component for this!

[julien-nc]

@dartcafe I made a similar component like your RadioGroupDiv for the specific needs of a project: https://github.com/eneiluj/integration_visavid/blob/master/src/components/RadioElementSet.vue
But I guess passing the options as an array of objects makes prop validation harder and gives less control than the suggestion of @raimund-schluessler to insert the group content in the default slot.

@marcoambrosini So we have multiple ways to go. Which one makes more sense?

• Leave the CheckboxRadioSwitch component untouched and create 2 new components: CheckboxRadioSwitchButton and CheckboxRadioSwitchButtonGroup. But CheckboxRadioSwitchButton would repeat most of CheckboxRadioSwitch's code...
• Merge this PR and implement CheckboxRadioSwitchGroup
• Any other idea?

[skjnldsv]

• Leave the CheckboxRadioSwitch component untouched and create 2 new components: CheckboxRadioSwitchButton and CheckboxRadioSwitchButtonGroup. But CheckboxRadioSwitchButton would repeat most of CheckboxRadioSwitch's code...

Yeah, I'm not in favour.
This component ships fancy utils and methods about grouping, naming, events etc etc.
That would just duplicate the code.

I would be ok creating a CheckboxRadioSwitchGroup, that would be easy to check the parent from the CheckboxRadioSwitch component just to see which direction is the prop set.

Let's keep it simple people! 🚀