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

[checkbox] Add helper support #3749

Closed
knoobie opened this issue Apr 28, 2022 · 7 comments · Fixed by #7285
Closed

[checkbox] Add helper support #3749

knoobie opened this issue Apr 28, 2022 · 7 comments · Fixed by #7285
Assignees
Labels
enhancement New feature or request needs design Design is needed vaadin-checkbox

Comments

@knoobie
Copy link
Contributor

knoobie commented Apr 28, 2022

Describe your motivation

Currently the checkbox does not support helper text/components.

Example: complex search form with a checkbox Enable phonetic search and a helper text "Lorem Ipsum... Phonetic search means.. Lorem Ipsum..".

Describe the solution you'd like

The checkbox should have support for the helper feature like every other component.

@web-padawan web-padawan added enhancement New feature or request vaadin-checkbox labels Apr 28, 2022
@yuriy-fix yuriy-fix added the needs design Design is needed label May 3, 2022
@yuriy-fix
Copy link
Contributor

Dear @knoobie,
have you considered wrapping checkbox with custom-field?
It could make sense to have one checkbox within checkbox-group.

@knoobie
Copy link
Contributor Author

knoobie commented May 3, 2022

@yuriy-fix I haven't tried any alternatives - both could probably work somehow.

@rolfsmeds
Copy link
Contributor

rolfsmeds commented Feb 27, 2024

This needs to be planned together with required state support (which implicitly brings with it error message support) and validation support.

In addition to a bit of UI design to figure out exactly how errors, helpers and the validation indicator should look in the Checkbox, and probably some dilemmas regarding validation, the bigger problem is how to provide a place in the component's DOM for these.

The required indicator should be pretty straight forward: we could just add an element for it after the label wrapper:
<span part="required-indicator" aria-hidden="true"></span>

As for helper, and error, other input fields have a <div class="vaadin-field-container"> that wraps the field, the error and the helper (i.e. everything but the label). The Checkbox has a similar <div class="vaadin-checkbox-container"> wrapper around the visual checkbox, the invisible native input, and the label. That's already rendered as a css-grid, which means that we should be able to add a row to the css-grid to place them in without affecting the rest of the DOM or any other styling.

Another issue that should be tackled is how to deal with required checkboxes in a Checkbox Group. Presumably the group's validation should be affected by the presence of required checkboxes so that the group itself becomes required, and is invalidated if one or more required checkbox within it is unchecked.

Ping @web-padawan for thoughts.

@web-padawan
Copy link
Member

Personally, I would prefer that we keep vaadin-checkbox in its current shape and introduce vaadin-checkbox-field.
This is basically inspired by the following comment by @jouni: vaadin/vaadin-checkbox#188 (comment)

IMO, this option would let us avoid breaking changes on the web component side, so that the feature could land in 24.5.

Also, we would then be able to keep using vaadin-checkbox in vaadin-checkbox-group to address this concern:

Another issue that should be tackled is how to deal with required checkboxes in a Checkbox Group.

@knoobie
Copy link
Contributor Author

knoobie commented Feb 27, 2024

I don't mind any of the mentioned options, we would be fine with both directions.

The additional component could also be introduced on the java side; if you want to - e.g. CheckboxField if you are worried that changing the original java class might cause other breaking changes for those user groups. People that wanna use "the real" checkbox could opt in to replace their old class and / or it might also be possible to deprecate the old class in favor of the new class in the long run (purely java talking)

@rolfsmeds
Copy link
Contributor

I think it would be best to analyze what breaking changes would need to be introduced by these additions. On the DOM/CSS side I don't see the need for any, and the additional APIs seem like additions rather than anything affecting existing features.

Perhaps also worth pointing out that the native required state is available in the native <input type="checkbox">.

I'm not against a separate component per se, but I am a bit worried about creating clutter and causing confusion by having two distinct components that do basically the same thing. (Admittedly we already have several components that, arguably, fall into that category, but I don't think we should strive to add more.)

@rolfsmeds
Copy link
Contributor

Acceptance Criteria: vaadin/platform#5190

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs design Design is needed vaadin-checkbox
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants