Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with radio group component #965

Closed
xdev1 opened this issue Oct 20, 2022 · 2 comments
Closed

Issues with radio group component #965

xdev1 opened this issue Oct 20, 2022 · 2 comments
Assignees
Labels
bug Things that aren't working right in the library.

Comments

@xdev1
Copy link
Contributor

xdev1 commented Oct 20, 2022

I open this as a bug, as I consider this as a conceptional bug:
The "radio group" component is currently conceptional completely different than other form control components in Shoelace, which really should not be, IMHO.
If the attribute label is set (or the corresponding slot is used) this should be handled as for (almost) all other SL form control components, means: the label shall be shown directly above the input elements.

This is the way it is for example done in Microsoft Fluent UI's ChoiceGroup component:

image

... or in Vaadin's vaadin-radio-group component:

image

If you currently want to show the label in SL's radio group than you have to set attribute fieldset, but this will show an ugly fieldset frame around it, and you have to use CSS to fix that appearance if you don't like or need it.

Horizontal label layout is not really easy to implement. It may be possible (I have not tried it) but it will be in any cases different and not as easy as described here.

Stuff like this is not easy to implement because of the mentioned issues:

image

Proposal:

  • If attribute fieldet is set then render as already done (Is there really demand for this feature? Anyway...).
  • If attribute fieldset is not set then render the radio group without <fieldset> element. Instead render everything similar to other SL form controls, using the usual CSS parts form-control, form-control-label, form-control-input.
  • For use cases where no visual label shall be used, provide an additional attribute to set the "hidden label" (or something similar) for accessibility reasons.
@xdev1 xdev1 added the bug Things that aren't working right in the library. label Oct 20, 2022
@claviska
Copy link
Member

This is a valid point. Give me some time to circle back and address this, as I have some other form control-related improvements I'd like to make as well.

@claviska claviska added this to the 2.0.0 stable milestone Nov 8, 2022
@claviska
Copy link
Member

claviska commented Nov 9, 2022

I've reworked radio group to act more like inputs and textareas, where they have a label, the control, and optional help text. The styling was also improved to remove the border by default, as Shoelace shouldn't have an opinion here. Users can add it back by targeting the form-control and form-control__label parts.

These changes will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

No branches or pull requests

2 participants