carbon-design-system / carbon-components-svelte

Svelte implementation of the Carbon Design System
https://svelte.carbondesignsystem.com
Apache License 2.0
2.71k stars 261 forks source link

fix(multi-select): render checkboxes for form data #1835

Closed theetrain closed 1 year ago

theetrain commented 1 year ago

Fixes #1742

MultiSelect up til now wasn't rendering hidden inputs in order to be submittable within a <form>. With this enhancement:

  1. MultiSelect renders all items as <input type="hidden" name="some-id" value="true" /> using value="true" for checked items and value="false" for unchecked items by default.
  2. New prop selectedOnly only renders hidden inputs for user-selected items.
  3. New prop combineValues renders a single hidden input with comma-separated values, or using a consumer-provided delimiter.
  4. Prop itemToInput is enhanced to perform double duty for user-visible checkboxes as well as the hidden inputs, mainly to help override name and value attributes among the hidden inputs.
  5. Use CSS to show and hide MultiSelect values so that their checkboxes are form-submittable

Feature (3) was a personal need in order to submit an array of values within a form, but I felt the default behaviour should be checkbox-like as per point (1) assuming that's what consumers of MultiSelect would expect.

See included documentation for more examples.

theetrain commented 1 year ago

Thanks @metonym, though I think I had a new realization: if I conditionally hide the ListBox using display: none CSS, its checkbox inputs will still be included as part of the submitted form data:

Reference:

https://github.com/carbon-design-system/carbon-components-svelte/blob/276465aa9d5eb7a0bc1459d6dfd7cf3f8b1bdd7a/src/MultiSelect/MultiSelect.svelte#L501-L506

Refactor:

-  {#if open}
+  <div class:hidden="{!open}">
      <ListBoxMenu
        aria-label="{ariaLabel}"
        id="{id}"
        aria-multiselectable="true"
      >
<!-- stuff -->

+<style>
+ .hidden {
+   display: none;
+ }
+</style>

Minimal demo: https://www.sveltelab.dev/ibxj8ki83lee147

This is great for progressive enhancement down the road, once the popover API becomes stable. With that said, should I refactor to use the above logic? I can probably remove the new props as well and have my own project make use of the checkboxes; leveraging itemToInput to render custom name attributes for my needs.

What do you think?

metonym commented 1 year ago

That approach seems valid.

I'll admit that for one-line style changes, I have a slight preference for using the style: directive.