fix(ld-checkbox): prop forwarding and indeterminate state

emdgroup-liquid · Dec 2, 2021 · 572e711 · 572e711
1 parent bbf9a29
commit 572e711
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 64 deletions.
diff --git a/src/liquid/components/ld-checkbox/ld-checkbox.css b/src/liquid/components/ld-checkbox/ld-checkbox.css
@@ -57,9 +57,7 @@
       }
     }
 
-    /* For whatever reason both selectors are required to work with both, web and css component */
-    &:indeterminate,
-    &[indeterminate] {
+    &:indeterminate {
       ~ .ld-checkbox__box::before {
         content: '';
         position: absolute;

diff --git a/src/liquid/components/ld-checkbox/ld-checkbox.tsx b/src/liquid/components/ld-checkbox/ld-checkbox.tsx
@@ -24,7 +24,7 @@ export class LdCheckbox implements InnerFocusable {
   /** Automatically focus the form control when the page is loaded. */
   @Prop() autofocus?: boolean
 
-  /** The input value. */
+  /** Indicates whether the checkbox is checked. */
   @Prop({ mutable: true, reflect: true }) checked: boolean
 
   /** Disabled state of the checkbox. */
@@ -42,9 +42,6 @@ export class LdCheckbox implements InnerFocusable {
   /** Set this property to `true` in order to mark the checkbox visually as invalid. */
   @Prop() invalid: boolean
 
-  /** Value of the id attribute of the `<datalist>` of autocomplete options. */
-  @Prop() list?: string
-
   /** Display mode. */
   @Prop() mode?: 'highlight' | 'danger'
 
@@ -79,16 +76,12 @@ export class LdCheckbox implements InnerFocusable {
   }
 
   @Watch('checked')
-  @Watch('form')
-  @Watch('indeterminate')
   @Watch('name')
-  @Watch('required')
   @Watch('value')
   updateHiddenInput() {
     const outerForm = this.el.closest('form')
     if (!this.hiddenInput && this.name && (outerForm || this.form)) {
-      this.hiddenInput = document.createElement('input')
-      this.el.appendChild(this.hiddenInput)
+      this.createHiddenInput()
     }
 
     if (this.hiddenInput) {
@@ -100,8 +93,6 @@ export class LdCheckbox implements InnerFocusable {
 
       this.hiddenInput.name = this.name
       this.hiddenInput.checked = this.checked
-      this.hiddenInput.required = this.required
-      this.hiddenInput.indeterminate = this.indeterminate
 
       if (this.value) {
         this.hiddenInput.value = this.value
@@ -122,6 +113,15 @@ export class LdCheckbox implements InnerFocusable {
     }
   }
 
+  private createHiddenInput() {
+    this.hiddenInput = document.createElement('input')
+    this.hiddenInput.type = 'checkbox'
+    this.hiddenInput.style.visibility = 'hidden'
+    this.hiddenInput.style.position = 'absolute'
+    this.hiddenInput.style.pointerEvents = 'none'
+    this.el.appendChild(this.hiddenInput)
+  }
+
   private handleBlur(ev) {
     setTimeout(() => {
       this.el.dispatchEvent(ev)
@@ -158,31 +158,20 @@ export class LdCheckbox implements InnerFocusable {
       this.autocomplete = outerForm.getAttribute('autocomplete')
     }
 
-    if (outerForm || this.form) {
-      if (this.name) {
-        this.hiddenInput = document.createElement('input')
-        this.hiddenInput.required = this.required
-        this.hiddenInput.type = 'checkbox'
-        this.hiddenInput.style.visibility = 'hidden'
-        this.hiddenInput.style.position = 'absolute'
-        this.hiddenInput.style.pointerEvents = 'none'
-        this.hiddenInput.checked = this.checked
-        this.hiddenInput.name = this.name
-
-        if (this.form) {
-          this.hiddenInput.setAttribute('form', this.form)
-        }
-
-        if (this.indeterminate) {
-          this.hiddenInput.indeterminate = this.indeterminate
-        }
+    if (this.name && (outerForm || this.form)) {
+      this.createHiddenInput()
+      this.hiddenInput.checked = this.checked
+      this.hiddenInput.name = this.name
 
-        if (this.value) {
-          this.hiddenInput.value = this.value
-        }
+      if (this.form) {
+        this.hiddenInput.setAttribute('form', this.form)
+      }
 
-        this.el.appendChild(this.hiddenInput)
+      if (this.value) {
+        this.hiddenInput.value = this.value
       }
+
+      this.el.appendChild(this.hiddenInput)
     }
   }
 
@@ -208,7 +197,7 @@ export class LdCheckbox implements InnerFocusable {
           onFocus={this.handleFocus}
           ref={(ref) => (this.input = ref)}
           type="checkbox"
-          {...cloneAttributes(this.el)}
+          {...cloneAttributes(this.el, ['autocomplete', 'tone', 'mode'])}
           disabled={this.disabled}
           checked={this.checked}
         />

diff --git a/src/liquid/components/ld-checkbox/readme.md b/src/liquid/components/ld-checkbox/readme.md
@@ -178,7 +178,7 @@ This component can be used in conjunction with the [`ld-label`](components/ld-la
 
 If the `indeterminate` attribute is present on the `ld-checkbox` component, the checkbox's value is neither `true` nor `false`, but is instead _indeterminate_, meaning that its state cannot be determined or stated in pure binary terms. This may happen, for instance, if the state of the checkbox depends on multiple other checkboxes, and those checkboxes have different values.
 
-> **Note**: When using the CSS Component you need to take care of removing the indeterminate prop yourself.  
+> **Note**: When using the CSS Component you need to take care of setting the indeterminate **prop** on the input element with JavaScript.  
 
 {% example 'html', false, false, 'light' %}
 <ld-checkbox indeterminate></ld-checkbox>
@@ -187,14 +187,8 @@ If the `indeterminate` attribute is present on the `ld-checkbox` component, the
 
 <!-- CSS component -->
 
-<script>
-document.addEventListener('input', ev => {
-  ev.target.removeAttribute('indeterminate')
-})
-</script>
-
-<div class="ld-checkbox">
-  <input type="checkbox" indeterminate>
+<div id="example-indeterminate-1" class="ld-checkbox">
+  <input type="checkbox">
   <svg
     class="ld-checkbox__check"
     width="14"
@@ -214,8 +208,8 @@ document.addEventListener('input', ev => {
   <div class="ld-checkbox__box"></div>
 </div>
 
-<div class="ld-checkbox">
-  <input type="checkbox" indeterminate disabled>
+<div id="example-indeterminate-2" class="ld-checkbox">
+  <input type="checkbox" disabled>
   <svg
     class="ld-checkbox__check"
     width="14"
@@ -235,6 +229,11 @@ document.addEventListener('input', ev => {
   <div class="ld-checkbox__box"></div>
 </div>
 
+<script>
+document.querySelectorAll('#example-indeterminate-1 input, #example-indeterminate-2 input')
+  .forEach(input => input.indeterminate = true)
+</script>
+
 {% endexample %}
 
 ### Dark
@@ -653,13 +652,12 @@ The `ld-checkbox` Web Component provides a low level API for integrating the com
 | --------------- | --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------- | ----------- |
 | `autocomplete`  | `autocomplete`  | Hint for form autofill feature.                                                                                                                                                      | `string`                  | `undefined` |
 | `autofocus`     | `autofocus`     | Automatically focus the form control when the page is loaded.                                                                                                                        | `boolean`                 | `undefined` |
-| `checked`       | `checked`       | The input value.                                                                                                                                                                     | `boolean`                 | `undefined` |
+| `checked`       | `checked`       | Indicates whether the checkbox is checked.                                                                                                                                           | `boolean`                 | `undefined` |
 | `disabled`      | `disabled`      | Disabled state of the checkbox.                                                                                                                                                      | `boolean`                 | `undefined` |
 | `form`          | `form`          | Associates the control with a form element.                                                                                                                                          | `string`                  | `undefined` |
 | `indeterminate` | `indeterminate` | Set this property to `true` to indicate that the checkbox's value is neither true nor false. The prop is removed automatically as soon as the checkbox is clicked (if not disabled). | `boolean`                 | `undefined` |
 | `invalid`       | `invalid`       | Set this property to `true` in order to mark the checkbox visually as invalid.                                                                                                       | `boolean`                 | `undefined` |
 | `key`           | `key`           | for tracking the node's identity when working with lists                                                                                                                             | `string \| number`        | `undefined` |
-| `list`          | `list`          | Value of the id attribute of the `<datalist>` of autocomplete options.                                                                                                               | `string`                  | `undefined` |
 | `mode`          | `mode`          | Display mode.                                                                                                                                                                        | `"danger" \| "highlight"` | `undefined` |
 | `name`          | `name`          | Used to specify the name of the control.                                                                                                                                             | `string`                  | `undefined` |
 | `readonly`      | `readonly`      | The value is not editable.                                                                                                                                                           | `boolean`                 | `undefined` |

diff --git a/src/liquid/components/ld-checkbox/test/__snapshots__/ld-checkbox.spec.ts.snap b/src/liquid/components/ld-checkbox/test/__snapshots__/ld-checkbox.spec.ts.snap
@@ -3,7 +3,7 @@
 exports[`ld-checkbox creates hidden input field, if inside a form 1`] = `
 <ld-checkbox class="ld-checkbox" name="example" part="root">
   <mock:shadow-root>
-    <input part="input focusable" type="checkbox">
+    <input name="example" part="input focusable" type="checkbox">
     <svg class="ld-checkbox__check" fill="none" height="14" part="check" viewBox="0 0 14 14" width="14" xmlns="http://www.w3.org/2000/svg">
       <path d="M12 4L5.40795 10L2 6.63964" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="3"></path>
     </svg>

diff --git a/src/liquid/components/ld-checkbox/test/ld-checkbox.e2e.ts b/src/liquid/components/ld-checkbox/test/ld-checkbox.e2e.ts
@@ -286,10 +286,15 @@ describe('ld-checkbox', () => {
     it('css component', async () => {
       const page = await getPageWithContent(
         `<div class="ld-checkbox">
-          <input type="checkbox" indeterminate></input>${checkAndBox}
+          <input type="checkbox"></input>${checkAndBox}
         </div>`,
         { components: LdCheckbox }
       )
+      await page.evaluate(() => {
+        ;(
+          document.querySelector('input[type="checkbox"]') as HTMLInputElement
+        ).indeterminate = true
+      })
       const results = await page.compareScreenshot()
       expect(results).toMatchScreenshot()
     })
@@ -309,6 +314,11 @@ describe('ld-checkbox', () => {
         </div>`,
         { components: LdCheckbox }
       )
+      await page.evaluate(() => {
+        ;(
+          document.querySelector('input[type="checkbox"]') as HTMLInputElement
+        ).indeterminate = true
+      })
       const results = await page.compareScreenshot()
       expect(results).toMatchScreenshot()
     })

diff --git a/src/liquid/components/ld-checkbox/test/ld-checkbox.spec.ts b/src/liquid/components/ld-checkbox/test/ld-checkbox.spec.ts
@@ -1,4 +1,5 @@
 import { newSpecPage } from '@stencil/core/testing'
+jest.mock('../../../utils/cloneAttributes')
 import { LdCheckbox } from '../ld-checkbox'
 
 describe('ld-checkbox', () => {
@@ -129,25 +130,22 @@ describe('ld-checkbox', () => {
   it('sets initial state on hidden input', async () => {
     const page = await newSpecPage({
       components: [LdCheckbox],
-      html: '<form><ld-checkbox name="example" checked required /></form>',
+      html: '<form><ld-checkbox name="example" checked /></form>',
     })
     const ldCheckbox = page.root
     expect(ldCheckbox.querySelector('input')).toHaveProperty('name', 'example')
-    expect(ldCheckbox.querySelector('input')).toHaveProperty('required', true)
   })
 
   it('updates hidden input field', async () => {
     const { root, waitForChanges } = await newSpecPage({
       components: [LdCheckbox],
-      html: `<form><ld-checkbox name="example" indeterminate /></form>`,
+      html: `<form><ld-checkbox name="example" /></form>`,
     })
     const ldCheckbox = root
     await waitForChanges()
 
     expect(ldCheckbox.querySelector('input')).toHaveProperty('name', 'example')
     expect(ldCheckbox.querySelector('input')).toHaveProperty('checked', false)
-    expect(ldCheckbox.querySelector('input')).toHaveProperty('required', false)
-    expect(ldCheckbox.querySelector('input').indeterminate).toEqual(true)
 
     ldCheckbox.setAttribute('name', 'test')
     await waitForChanges()
@@ -168,12 +166,6 @@ describe('ld-checkbox', () => {
     await waitForChanges()
 
     expect(ldCheckbox.querySelector('input')).toHaveProperty('checked', true)
-    expect(ldCheckbox.querySelector('input').indeterminate).toEqual(undefined)
-
-    ldCheckbox.setAttribute('required', '')
-    await waitForChanges()
-
-    expect(ldCheckbox.querySelector('input')).toHaveProperty('required', true)
 
     ldCheckbox.setAttribute('value', 'test')
     await waitForChanges()
@@ -191,7 +183,7 @@ describe('ld-checkbox', () => {
   it('uses hidden input field with referenced form', async () => {
     const { root, waitForChanges } = await newSpecPage({
       components: [LdCheckbox],
-      html: '<ld-checkbox name="example" indeterminate form="yolo" />',
+      html: '<ld-checkbox name="example" form="yolo" />',
     })
     const ldCheckbox = root
     await waitForChanges()