fix(quantity): review

ovh · Jul 29, 2024 · 6fe59fd · 6fe59fd
1 parent 4a60bd5
commit 6fe59fd
Show file tree

Hide file tree

Showing 13 changed files with 146 additions and 143 deletions.
diff --git a/packages/ods/src/components/input/src/components/ods-input/ods-input.scss b/packages/ods/src/components/input/src/components/ods-input/ods-input.scss
@@ -108,6 +108,10 @@ $ods-input-input-padding-right: calc($ods-input-actions-button-width + $ods-inpu
       }
     }
 
+    &:not(:disabled)#{$input}--error {
+      border-color: var(--ods-form-element-border-color-critical);
+    }
+
     &:disabled {
       ~ .ods-input__actions {
         cursor: not-allowed;

diff --git a/packages/ods/src/components/input/src/index.html b/packages/ods/src/components/input/src/index.html
@@ -23,6 +23,11 @@
   <ods-input value="Disabled" is-disabled is-clearable>
   </ods-input>
 
+  <p>Disabled & Error</p>
+  <ods-input has-error is-disabled>
+  </ods-input>
+
+
   <p>Error</p>
   <ods-input has-error>
   </ods-input>

diff --git a/packages/ods/src/components/quantity/src/components/ods-quantity/ods-quantity.scss b/packages/ods/src/components/quantity/src/components/ods-quantity/ods-quantity.scss
@@ -8,18 +8,11 @@
   &__button {
     $button: &;
 
-    &:not(#{$button}--disabled, #{$button}--error)::part(button) {
-      background-color: var(--ods-color-primary-000);
-      color: var(--ods-color-primary-500);
-
-      &:active {
-        border-color: var(--ods-color-primary-700);
-        background-color: var(--ods-color-primary-100);
-        color: var(--ods-color-primary-700);
-      }
+    &::part(button) {
+      position: inherit;
 
       &:focus-visible {
-        outline-offset: -2px;
+        position: relative;
       }
     }
 
@@ -33,23 +26,24 @@
       border-bottom-left-radius: 0;
     }
 
-    &--disabled::part(button) {
-      border-color: var(--ods-form-element-border-color-default);
-    }
-
-    &--error::part(button) {
-      background-color: var(--ods-color-critical-000);
-      color: var(--ods-color-critical-500);
+    &--readonly::part(button) {
+      pointer-events: none;
     }
   }
 
   &__input {
+    position: inherit;
+
     &::part(input) {
+      border-right: 0;
+      border-left: 0;
       border-radius: 0;
+      padding: 4px;
       width: 50px;
+      text-align: center;
 
       &:focus-visible {
-        outline-offset: -2px;
+        position: relative;
       }
     }
   }

diff --git a/packages/ods/src/components/quantity/src/components/ods-quantity/ods-quantity.tsx b/packages/ods/src/components/quantity/src/components/ods-quantity/ods-quantity.tsx
@@ -1,5 +1,5 @@
 import { AttachInternals, Component, Event, type EventEmitter, type FunctionalComponent, Host, Method, Prop, h } from '@stencil/core';
-import { ODS_BUTTON_COLOR, ODS_BUTTON_SIZE } from '../../../../button/src';
+import { ODS_BUTTON_COLOR, ODS_BUTTON_SIZE, ODS_BUTTON_VARIANT } from '../../../../button/src';
 import { ODS_ICON_NAME } from '../../../../icon/src';
 import { ODS_INPUT_TYPE, type OdsInput, type OdsInputValueChangeEvent } from '../../../../input/src';
 import { isAddButtonDisabled, isMinusButtonDisabled, setFormValue } from '../../controller/ods-quantity';
@@ -21,8 +21,8 @@ export class OdsQuantity {
   @Prop({ reflect: true }) public defaultValue?: number;
   @Prop({ reflect: true }) public hasError: boolean = false;
   @Prop({ reflect: true }) public isDisabled: boolean = false;
-  @Prop({ reflect: true }) public isReadonly?: boolean = false;
-  @Prop({ reflect: true }) public isRequired?: boolean = false;
+  @Prop({ reflect: true }) public isReadonly: boolean = false;
+  @Prop({ reflect: true }) public isRequired: boolean = false;
   @Prop({ reflect: true }) public max?: number;
   @Prop({ reflect: true }) public min?: number;
   @Prop({ reflect: true }) public name!: string;
@@ -93,17 +93,16 @@ export class OdsQuantity {
         <ods-button
           class={{
             'ods-quantity__button': true,
-            'ods-quantity__button--disabled': isMinusButtonDisabled(this.isDisabled, this.value, this.min),
-            'ods-quantity__button--error': this.hasError,
+            'ods-quantity__button--readonly': this.isReadonly,
           }}
           color={ this.hasError ? ODS_BUTTON_COLOR.critical : ODS_BUTTON_COLOR.primary }
           exportparts="button:button_minus"
           isDisabled={ isMinusButtonDisabled(this.isDisabled, this.value, this.min) }
           icon={ ODS_ICON_NAME.minus }
           label=""
           onClick={ () => this.decrement() }
-          onFocus={ () => this.odsFocus.emit() }
-          size={ ODS_BUTTON_SIZE.sm }>
+          size={ ODS_BUTTON_SIZE.sm }
+          variant={ ODS_BUTTON_VARIANT.outline }>
         </ods-button>
         <ods-input
           ariaLabel={ this.ariaLabel }
@@ -128,17 +127,16 @@ export class OdsQuantity {
         <ods-button
           class={{
             'ods-quantity__button': true,
-            'ods-quantity__button--disabled': isAddButtonDisabled(this.isDisabled, this.value, this.max),
-            'ods-quantity__button--error': this.hasError,
+            'ods-quantity__button--readonly': this.isReadonly,
           }}
           color={ this.hasError ? ODS_BUTTON_COLOR.critical : ODS_BUTTON_COLOR.primary }
           exportparts="button:button_add"
           isDisabled={ isAddButtonDisabled(this.isDisabled, this.value, this.max) }
           icon={ ODS_ICON_NAME.add }
           label=""
           onClick={ () => this.increment() }
-          onFocus={ () => this.odsFocus.emit() }
-          size={ ODS_BUTTON_SIZE.sm }>
+          size={ ODS_BUTTON_SIZE.sm }
+          variant={ ODS_BUTTON_VARIANT.outline }>
         </ods-button>
       </Host>
     );

diff --git a/packages/ods/src/components/quantity/src/controller/ods-quantity.ts b/packages/ods/src/components/quantity/src/controller/ods-quantity.ts
@@ -1,9 +1,9 @@
 function isAddButtonDisabled(isDisabled: boolean, value: number | null, max?: number): boolean {
-  return isDisabled || (max !== undefined && max === value);
+  return isDisabled || (max !== undefined && value !== null && max <= value);
 }
 
 function isMinusButtonDisabled(isDisabled: boolean, value: number | null, min?: number): boolean {
-  return isDisabled || (min !== undefined && min === value);
+  return isDisabled || (min !== undefined && value !== null && min >= value);
 }
 
 function setFormValue(internals: ElementInternals, value: number | null): void {

diff --git a/packages/ods/src/components/quantity/src/index.html b/packages/ods/src/components/quantity/src/index.html
@@ -38,6 +38,10 @@
   <ods-quantity has-error>
   </ods-quantity>
 
+  <p>Error & Disabled</p>
+  <ods-quantity has-error is-disabled>
+  </ods-quantity>
+
   <p>Disabled</p>
   <ods-quantity is-disabled>
   </ods-quantity>
@@ -47,7 +51,7 @@
   </ods-quantity>
 
   <p>Max</p>
-  <ods-quantity max="10" value="10">
+  <ods-quantity max="10" value="11">
   </ods-quantity>
 
   <p>Readonly</p>

diff --git a/packages/ods/src/components/quantity/tests/behaviour/ods-quantity.e2e.ts b/packages/ods/src/components/quantity/tests/behaviour/ods-quantity.e2e.ts
@@ -24,7 +24,7 @@ describe('ods-quantity behaviour', () => {
 
   describe('method:clear', () => {
     it('should receive odsClear event', async() => {
-      await setup(`<ods-quantity value="${0}"></ods-quantity>`);
+      await setup('<ods-quantity value="0"></ods-quantity>');
       const odsClearSpy = await page.spyOnEvent('odsClear');
       await el.callMethod('clear');
       await page.waitForChanges();
@@ -36,7 +36,7 @@ describe('ods-quantity behaviour', () => {
   describe('method:reset', () => {
     it('should receive odsReset event', async() => {
       const defaultValue = 5;
-      await setup(`<ods-quantity value="${8}" default-value="${defaultValue}"></ods-quantity>`);
+      await setup(`<ods-quantity value="8" default-value="${defaultValue}"></ods-quantity>`);
       const odsResetSpy = await page.spyOnEvent('odsReset');
       await el.callMethod('reset');
       await page.waitForChanges();
@@ -47,7 +47,7 @@ describe('ods-quantity behaviour', () => {
 
   describe('button minus', () => {
     it('should decrement value', async() => {
-      await setup(`<ods-quantity value="${0}"></ods-quantity>`);
+      await setup('<ods-quantity value="0"></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonMinus.click();
       await page.waitForChanges();
@@ -56,7 +56,7 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should decrement value with step', async() => {
-      await setup(`<ods-quantity value="${0}" step="${10}"></ods-quantity>`);
+      await setup('<ods-quantity value="0" step="10"></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonMinus.click();
       await page.waitForChanges();
@@ -65,7 +65,7 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should not decrement value with is-disabled', async() => {
-      await setup(`<ods-quantity value="${0}" is-disabled></ods-quantity>`);
+      await setup('<ods-quantity value="0" is-disabled></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonMinus.click();
       await page.waitForChanges();
@@ -74,7 +74,7 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should not decrement value with is-readonly', async() => {
-      await setup(`<ods-quantity value="${0}" is-readonly></ods-quantity>`);
+      await setup('<ods-quantity value="0" is-readonly></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonMinus.click();
       await page.waitForChanges();
@@ -83,18 +83,27 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should not decrement value with min equal to value', async() => {
-      await setup(`<ods-quantity value="${0}" min="${0}"></ods-quantity>`);
+      await setup('<ods-quantity value="0" min="0"></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonMinus.click();
       await page.waitForChanges();
       expect(await el.getProperty('value')).toBe(0);
       expect(odsChangeSpy).toHaveReceivedEventTimes(0);
     });
+
+    it('should not decrement value with value inferior to min', async() => {
+      await setup('<ods-quantity value="4" min="5"></ods-quantity>');
+      const odsChangeSpy = await page.spyOnEvent('odsChange');
+      await buttonMinus.click();
+      await page.waitForChanges();
+      expect(await el.getProperty('value')).toBe(4);
+      expect(odsChangeSpy).toHaveReceivedEventTimes(0);
+    });
   });
 
   describe('button add', () => {
     it('should increment value', async() => {
-      await setup(`<ods-quantity value="${0}"></ods-quantity>`);
+      await setup('<ods-quantity value="0"></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonAdd.click();
       await page.waitForChanges();
@@ -103,7 +112,7 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should increment value with step', async() => {
-      await setup(`<ods-quantity value="${0}" step="${10}"></ods-quantity>`);
+      await setup('<ods-quantity value="0" step="10"></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonAdd.click();
       await page.waitForChanges();
@@ -112,7 +121,7 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should not increment value with is-disabled', async() => {
-      await setup(`<ods-quantity value="${0}" is-disabled></ods-quantity>`);
+      await setup('<ods-quantity value="0" is-disabled></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonAdd.click();
       await page.waitForChanges();
@@ -121,21 +130,75 @@ describe('ods-quantity behaviour', () => {
     });
 
     it('should not increment value with is-readonly', async() => {
-      await setup(`<ods-quantity value="${0}" is-readonly></ods-quantity>`);
+      await setup('<ods-quantity value="0" is-readonly></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonAdd.click();
       await page.waitForChanges();
       expect(await el.getProperty('value')).toBe(0);
       expect(odsChangeSpy).toHaveReceivedEventTimes(0);
     });
 
-    it('should not decrement value with max equal to value', async() => {
-      await setup(`<ods-quantity value="${10}" max="${10}"></ods-quantity>`);
+    it('should not increment value with max equal to value', async() => {
+      await setup('<ods-quantity value="10" max="10"></ods-quantity>');
       const odsChangeSpy = await page.spyOnEvent('odsChange');
       await buttonAdd.click();
       await page.waitForChanges();
       expect(await el.getProperty('value')).toBe(10);
       expect(odsChangeSpy).toHaveReceivedEventTimes(0);
     });
+
+    it('should not increment value with value superior to max', async() => {
+      await setup('<ods-quantity value="11" max="10"></ods-quantity>');
+      const odsChangeSpy = await page.spyOnEvent('odsChange');
+      await buttonAdd.click();
+      await page.waitForChanges();
+      expect(await el.getProperty('value')).toBe(11);
+      expect(odsChangeSpy).toHaveReceivedEventTimes(0);
+    });
+  });
+
+  describe('Form', () => {
+    it('should get form data with button type submit  after increment quantity', async() => {
+      await setup(`<form method="get">
+        <ods-quantity name="ods-quantity" value="0"></ods-quantity>
+        <button type="reset">Reset</button>
+        <button type="submit">Submit</button>
+      </form>`);
+      await buttonAdd.click();
+      const submitButton = await page.find('button[type="submit"]');
+      await submitButton.click();
+      await page.waitForNetworkIdle();
+      const url = new URL(page.url());
+      expect(url.searchParams.get('ods-quantity')).toBe('1');
+    });
+
+    it('should get form data with button type submit', async() => {
+      await setup(`<form method="get">
+        <ods-quantity name="ods-quantity" value="0"></ods-quantity>
+        <button type="reset">Reset</button>
+        <button type="submit">Submit</button>
+      </form>`);
+      const submitButton = await page.find('button[type="submit"]');
+      await submitButton.click();
+      await page.waitForNetworkIdle();
+      const url = new URL(page.url());
+      expect(url.searchParams.get('ods-quantity')).toBe('0');
+    });
+
+    it('should reset form with button type reset', async() => {
+      await setup(`<form method="get">
+        <ods-quantity name="ods-quantity" value="0"></ods-quantity>
+        <button type="reset">Reset</button>
+        <button type="submit">Submit</button>
+      </form>`);
+      const resetButton = await page.find('button[type="reset"]');
+      await resetButton.click();
+
+      const submitButton = await page.find('button[type="submit"]');
+      await submitButton.click();
+      await page.waitForNetworkIdle();
+      const url = new URL(page.url());
+      expect(url.searchParams.get('ods-quantity')).toBe('0');
+    });
   });
 });
diff --git a/packages/ods/src/components/quantity/tests/controller/ods-quantity.spec.ts b/packages/ods/src/components/quantity/tests/controller/ods-quantity.spec.ts
@@ -11,6 +11,9 @@ describe('ods-input controller', () => {
 
       // max === value
       expect(isAddButtonDisabled(false, 10, 10)).toBe(true);
+
+      // max < value
+      expect(isAddButtonDisabled(false, 11, 10)).toBe(true);
     });
 
     it('should return false', async() => {
@@ -29,6 +32,9 @@ describe('ods-input controller', () => {
 
       // min === value
       expect(isMinusButtonDisabled(false, 0, 0)).toBe(true);
+
+      // min > value
+      expect(isMinusButtonDisabled(false, 0, 1)).toBe(true);
     });
 
     it('should return false', async() => {

diff --git a/packages/ods/src/components/quantity/tests/navigation/ods-quantity.e2e.ts b/packages/ods/src/components/quantity/tests/navigation/ods-quantity.e2e.ts
@@ -28,6 +28,7 @@ describe('ods-quantity navigation', () => {
       await setup('<ods-quantity></ods-quantity>');
       const odsFocusSpy = await page.spyOnEvent('odsFocus');
       await page.keyboard.press('Tab');
+      await page.keyboard.press('Tab');
       expect(await isFocused()).toBe(true);
       expect(odsFocusSpy).toHaveReceivedEventTimes(1);
     });