fix: make disabled attributes work with false in react

Resolves #229, #235
emdgroup-liquid · Jan 18, 2022 · 5a3ee62 · 5a3ee62
1 parent 3ce2c10
commit 5a3ee62
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 34 deletions.
diff --git a/src/liquid/components/ld-input/ld-input.css b/src/liquid/components/ld-input/ld-input.css
@@ -351,8 +351,8 @@
 }
 
 @media (hover: hover) {
-  :host(:not(.ld-input--invalid):not([aria-disabled='true']):not([disabled]):hover:not(:focus-within)),
-  .ld-input:not(.ld-input--invalid):not([aria-disabled='true']):not([disabled]):hover:not(:focus-within) {
+  :host(:not(.ld-input--invalid):not([aria-disabled='true']):not(.ld-input--disabled):hover:not(:focus-within)),
+  .ld-input:not(.ld-input--invalid):not([aria-disabled='true']):not(.ld-input--disabled):hover:not(:focus-within) {
     &::before {
       box-shadow: inset 0 0 0 var(--ld-sp-2) var(--ld-input-border-col-hover);
     }
@@ -376,14 +376,14 @@
   background-color: var(--ld-input-bg-col-invalid-focus);
 }
 
-:host(.ld-input--invalid:not([disabled]):not([aria-disabled='true']):where(:not(:focus))),
-.ld-input--invalid:not([disabled]):not([aria-disabled='true']):where(:not(:focus)) {
+:host(.ld-input--invalid:not(.ld-input--disabled):not([aria-disabled='true']):where(:not(:focus))),
+.ld-input--invalid:not(.ld-input--disabled):not([aria-disabled='true']):where(:not(:focus)) {
   background-color: var(--ld-input-bg-col-invalid);
   color: var(--ld-input-text-col-invalid);
 }
 
-:host(.ld-input--invalid:not([disabled]):not([aria-disabled='true'])),
-.ld-input--invalid:not([disabled]):not([aria-disabled='true']) {
+:host(.ld-input--invalid:not(.ld-input--disabled):not([aria-disabled='true'])),
+.ld-input--invalid:not(.ld-input--disabled):not([aria-disabled='true']) {
   &::before {
     box-shadow: inset 0 0 0 var(--ld-sp-2) var(--ld-input-text-col-invalid);
   }
@@ -410,8 +410,8 @@
   }
 }
 
-:host(.ld-input--invalid:not([disabled]):not([aria-disabled='true']):focus-within),
-.ld-input--invalid:not([disabled]):not([aria-disabled='true']):focus-within {
+:host(.ld-input--invalid:not(.ld-input--disabled):not([aria-disabled='true']):focus-within),
+.ld-input--invalid:not(.ld-input--disabled):not([aria-disabled='true']):focus-within {
   background-color: var(--ld-input-bg-col-invalid-focus);
 
   > input,
@@ -420,9 +420,9 @@
   }
 }
 
-:host([disabled]),
+:host(.ld-input--disabled),
 :host([aria-disabled='true']),
-.ld-input[disabled],
+.ld-input.ld-input--disabled,
 .ld-input[aria-disabled='true'] {
   color: var(--ld-input-text-col-disabled);
   background-color: var(--ld-input-bg-col-disabled);
@@ -468,8 +468,8 @@
   left: var(--ld-input-padding-x-md);
   margin-right: 0;
 
-  :host([disabled]) &,
-  .ld-input[disabled] & {
+  :host(.ld-input--disabled) &,
+  .ld-input.ld-input--disabled & {
     opacity: 0.25;
   }
 }

diff --git a/src/liquid/components/ld-input/ld-input.tsx b/src/liquid/components/ld-input/ld-input.tsx
@@ -335,6 +335,7 @@ export class LdInput implements InnerFocusable, ClonesAttributes {
   render() {
     const cl = getClassNames([
       'ld-input',
+      this.disabled && `ld-input--disabled`,
       this.size && `ld-input--${this.size}`,
       this.tone && `ld-input--${this.tone}`,
       this.invalid && 'ld-input--invalid',

diff --git a/src/liquid/components/ld-input/readme.md b/src/liquid/components/ld-input/readme.md
@@ -52,11 +52,11 @@ By default, the `ld-input` component is of [type `text`](https://developer.mozil
 
 <!-- CSS component -->
 
-<div class="ld-input" disabled>
+<div class="ld-input ld-input--disabled">
   <input placeholder="Placeholder" disabled>
 </div>
 
-<div class="ld-input" disabled>
+<div class="ld-input ld-input--disabled">
   <input placeholder="Placeholder" value="Value" disabled>
 </div>
 {% endexample %}
@@ -70,14 +70,14 @@ By default, the `ld-input` component is of [type `text`](https://developer.mozil
 
 <!-- CSS component -->
 
-<div class="ld-input" disabled>
+<div class="ld-input ld-input--disabled">
   <input
     placeholder="Placeholder"
     aria-disabled="true"
     id="focusable-disabled-input-1">
 </div>
 
-<div class="ld-input" disabled>
+<div class="ld-input ld-input--disabled">
   <input
     placeholder="Placeholder"
     value="Value"
@@ -114,7 +114,7 @@ By default, the `ld-input` component is of [type `text`](https://developer.mozil
   <input placeholder="Placeholder">
 </div>
 
-<div class="ld-input ld-input--dark" disabled>
+<div class="ld-input ld-input--dark ld-input--disabled">
   <input placeholder="Placeholder" disabled>
 </div>
 {% endexample %}

diff --git a/src/liquid/components/ld-input/test/ld-input.e2e.ts b/src/liquid/components/ld-input/test/ld-input.e2e.ts
@@ -178,23 +178,23 @@ describe('ld-input', () => {
     // Disabled CSS component
     it(`css component disabled${toneStr}`, async () => {
       const page = await getPageWithContent(
-        `<div class="ld-input${toneModifier}" disabled><input disabled placeholder="Placeholder"></input>${cssIconComponent}</div>`,
+        `<div class="ld-input${toneModifier} ld-input--disabled"><input disabled placeholder="Placeholder"></input>${cssIconComponent}</div>`,
         { components: LdInput }
       )
       const results = await page.compareScreenshot()
       expect(results).toMatchScreenshot()
     })
     it(`css component disabled with value${toneStr}`, async () => {
       const page = await getPageWithContent(
-        `<div class="ld-input${toneModifier}" disabled><input disabled value="Value"></input>${cssIconComponent}</div>`,
+        `<div class="ld-input${toneModifier} ld-input--disabled"><input disabled value="Value"></input>${cssIconComponent}</div>`,
         { components: LdInput }
       )
       const results = await page.compareScreenshot()
       expect(results).toMatchScreenshot()
     })
     it(`css component disabled hover${toneStr}`, async () => {
       const page = await getPageWithContent(
-        `<div class="ld-input${toneModifier}" disabled><input disabled placeholder="Placeholder"></input>${cssIconComponent}</div>`,
+        `<div class="ld-input${toneModifier} ld-input--disabled"><input disabled placeholder="Placeholder"></input>${cssIconComponent}</div>`,
         { components: LdInput }
       )
       await page.hover('.ld-input')
@@ -203,7 +203,7 @@ describe('ld-input', () => {
     })
     it(`css component disabled focus${toneStr}`, async () => {
       const page = await getPageWithContent(
-        `<div class="ld-input${toneModifier}" disabled><input disabled placeholder="Placeholder"></input>${cssIconComponent}</div>`,
+        `<div class="ld-input${toneModifier} ld-input--disabled"><input disabled placeholder="Placeholder"></input>${cssIconComponent}</div>`,
         { components: LdInput }
       )
       await page.keyboard.press('Tab')

diff --git a/src/liquid/components/ld-label/ld-label.css b/src/liquid/components/ld-label/ld-label.css
@@ -86,7 +86,3 @@
   font: var(--ld-typo-label-m);
   line-height: 1;
 }
-
-:where(.ld-label[disabled]) {
-  color: var(--ld-label-disabled-col);
-}
diff --git a/src/liquid/components/ld-select/ld-option-internal/ld-option-internal.css b/src/liquid/components/ld-select/ld-option-internal/ld-option-internal.css
@@ -99,7 +99,7 @@
     }
   }
 
-  &:where(:not([disabled]):not([aria-disabled='true'])) {
+  &:where(:not(.ld-option-internal--disabled):not([aria-disabled='true'])) {
     :where(.ld-option-internal__check) {
       color: var(--ld-option-thm-col);
     }

diff --git a/src/liquid/components/ld-select/ld-option-internal/ld-option-internal.tsx b/src/liquid/components/ld-select/ld-option-internal/ld-option-internal.tsx
@@ -118,6 +118,7 @@ export class LdOptionInternal {
     return (
       <Host
         class={getClassNames([
+          this.disabled && 'ld-option-internal--disabled',
           this.hasFocus && 'ld-option-internal--focus-within',
           this.hasHover && 'ld-option-internal--hover-within',
         ])}

diff --git a/src/liquid/components/ld-select/ld-select.css b/src/liquid/components/ld-select/ld-select.css
@@ -26,6 +26,7 @@
   --ld-select-icon-size-sm: 1rem;
   --ld-select-icon-size-md: 1.25rem;
   --ld-select-icon-size-lg: 1.5rem;
+  --ld-select-trigger-line-height: 1.25;
 
   /* colors */
   --ld-select-col: var(--ld-col-neutral-900);
@@ -145,6 +146,7 @@
   align-items: center;
   justify-content: flex-end;
   font: var(--ld-typo-body-m);
+  line-height: var(--ld-select-trigger-line-height);
   border: 0;
   padding: var(--ld-select-padding-y-md) var(--ld-select-padding-x-md);
   border-radius: var(--ld-br-m);
@@ -153,7 +155,6 @@
   touch-action: manipulation;
   color: var(--ld-select-col);
   background-color: var(--ld-select-bg-col);
-  line-height: 1;
   text-align: left;
   appearance: none;
   -webkit-touch-callout: none;
@@ -334,6 +335,7 @@
 
 .ld-select__btn-trigger-text-wrapper {
   font: var(--ld-typo-label-m);
+  line-height: var(--ld-select-trigger-line-height);
   padding: var(--ld-sp-6) var(--ld-sp-8) var(--ld-sp-6) 0;
   height: calc(100% + var(--ld-sp-12));
   margin: calc(-1 * var(--ld-sp-6)) 0;
@@ -345,12 +347,12 @@
 .ld-select--sm > select,
 .ld-select--sm .ld-select__btn-trigger-text-wrapper {
   font: var(--ld-typo-body-s);
-  line-height: 1;
+  line-height: var(--ld-select-trigger-line-height);
 }
 .ld-select--lg > select,
 .ld-select--lg .ld-select__btn-trigger-text-wrapper {
   font: var(--ld-typo-body-l);
-  line-height: 1;
+  line-height: var(--ld-select-trigger-line-height);
 }
 
 .ld-select > select,
@@ -501,7 +503,7 @@
   display: none;
 }
 
-:where(.ld-select:not([disabled]):not([aria-disabled]):not(.ld-select--invalid)) {
+:where(.ld-select:not(.ld-select--disabled):not([aria-disabled]):not(.ld-select--invalid)) {
   .ld-select__btn-trigger {
     &:where(:focus:focus-visible) {
       box-shadow: inset 0 0 0 var(--ld-sp-2) var(--ld-select-thm-col);
@@ -563,7 +565,7 @@
   }
 }
 
-:where(.ld-select:not([disabled])):not(.ld-select--invalid) {
+:where(.ld-select:not(.ld-select--disabled)):not(.ld-select--invalid) {
   :where(select:not(:disabled)),
   :where(.ld-select__btn-trigger) {
     &:where(:focus:focus-visible) {
@@ -572,7 +574,7 @@
   }
 }
 
-:where(.ld-select:not([disabled]):not([aria-disabled]):not([aria-disabled='true'])) {
+:where(.ld-select:not(.ld-select--disabled):not([aria-disabled]):not([aria-disabled='true'])) {
   .ld-select__selection-list-more {
     color: var(--ld-select-thm-col);
   }

diff --git a/src/liquid/components/ld-select/ld-select.tsx b/src/liquid/components/ld-select/ld-select.tsx
@@ -946,6 +946,7 @@ export class LdSelect implements InnerFocusable {
 
     const cl = [
       'ld-select',
+      this.disabled && 'ld-select--disabled',
       this.size && `ld-select--${this.size}`,
       this.invalid && 'ld-select--invalid',
       this.expanded && 'ld-select--expanded',

diff --git a/src/liquid/utils/cloneAttributes.ts b/src/liquid/utils/cloneAttributes.ts
@@ -1,3 +1,7 @@
+function isFalsy(value: string | null | undefined) {
+  return value === undefined || value === null || value === 'false'
+}
+
 export function cloneAttributes(attributesToIgnore: string[] = []) {
   const attributesToIgnoreSet = new Set([
     'style',
@@ -11,7 +15,7 @@ export function cloneAttributes(attributesToIgnore: string[] = []) {
   // Get attributes not in props.
   const attributesToClone = {}
   for (const attr of this.el.attributes) {
-    if (attributesToIgnoreSet.has(attr.name)) {
+    if (attributesToIgnoreSet.has(attr.name) || isFalsy(attr.value)) {
       continue
     }
     const valueToClone = attr.value === '' ? true : attr.value
@@ -28,7 +32,7 @@ export function cloneAttributes(attributesToIgnore: string[] = []) {
       const { attributeName } = mutation
       if (!attributesToIgnoreSet.has(attributeName)) {
         const attrValue = this.el.getAttribute(attributeName)
-        if (attrValue === undefined || attrValue === null) {
+        if (isFalsy(attrValue)) {
           delete this.clonedAttributes[attributeName]
         } else {
           this.clonedAttributes[attributeName] = attrValue