fix(input): review

ovh · Jul 29, 2024 · 0dbf82e · 0dbf82e
1 parent 1e4b54a
commit 0dbf82e
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 43 deletions.
diff --git a/packages/ods/src/components/input/package.json b/packages/ods/src/components/input/package.json
@@ -11,7 +11,7 @@
     "lint:scss": "stylelint 'src/components/**/*.scss'",
     "lint:ts": "eslint '{src,tests}/**/*.{js,ts,tsx}'",
     "start": "stencil build --dev --watch --serve",
-    "test:e2e": "stencil test --e2e --config stencil.config.ts",
+    "test:e2e": "stencil test --e2e --config stencil.config.ts --max-workers=2",
     "test:e2e:ci": "tsc --noEmit && stencil test --e2e --ci --config stencil.config.ts",
     "test:spec": "stencil test --spec --config stencil.config.ts --coverage",
     "test:spec:ci": "tsc --noEmit && stencil test --config stencil.config.ts  --spec --ci --coverage"

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
@@ -6,24 +6,49 @@
   position: relative;
 }
 
-$ods-input__button-right: 4px;
+$ods-input-button-right: 4px;
+$ods-input-button-top: -1px;
 
 .ods-input {
   &__button {
+    $button: &;
+
     display: inline-flex;
     position: absolute;
-    top: 0;
-    right: $ods-input__button-right;
+    top: $ods-input-button-top;
+    right: $ods-input-button-right;
     border: none;
+    border-radius: 2px;
     background: none;
+    cursor: pointer;
     color: theme.$ods-color-primary-500;
 
     &:focus-visible {
       @include focus.ods-focus();
     }
 
-    &:not(:first-of-type) {
-      right: 30px;
+    &:hover {
+      background-color: theme.$ods-color-primary-100;
+    }
+
+    &:active {
+      background-color: theme.$ods-color-primary-200;
+    }
+
+    &__clearable {
+      color: theme.$ods-color-neutral-600;
+
+      &:has(+ #{$button}) {
+        right: 30px;
+      }
+
+      &:hover {
+        background-color: theme.$ods-color-neutral-100;
+      }
+
+      &:active {
+        background-color: theme.$ods-color-neutral-200;
+      }
     }
   }
 
@@ -32,7 +57,7 @@ $ods-input__button-right: 4px;
 
     @include input.ods-input();
 
-    &:not(#{$input}--error) {
+    &:not(#{$input}--error, #{$input}--disabled) {
       &:focus {
         border-color: theme.$ods-form-element-border-color-focused;
       }
@@ -55,12 +80,16 @@ $ods-input__button-right: 4px;
     &--error {
       border-color: theme.$ods-form-element-border-color-critical;
     }
+
+    &[type="number"] {
+      appearance: textfield;
+    }
   }
 
   &__spinner {
     position: absolute;
-    top: 0;
-    right: $ods-input__button-right;
+    top: $ods-input-button-top;
+    right: $ods-input-button-right;
     padding: 0 4px;
 
     &::part(spinner) {

diff --git a/packages/ods/src/components/input/src/components/ods-input/ods-input.tsx b/packages/ods/src/components/input/src/components/ods-input/ods-input.tsx
@@ -24,12 +24,12 @@ export class OdsInput {
   @Prop({ reflect: true }) public ariaLabelledby?: string;
   @Prop({ reflect: true }) public defaultValue?: string | number;
   @Prop({ reflect: true }) public hasError: boolean = false;
-  @Prop({ reflect: true }) public isClearable?: boolean = false;
+  @Prop({ reflect: true }) public isClearable: boolean = false;
   @Prop({ reflect: true }) public isDisabled: boolean = false;
-  @Prop({ reflect: true }) public isLoading?: boolean = false;
+  @Prop({ reflect: true }) public isLoading: boolean = false;
   @Prop({ mutable: true, reflect: true }) public isMasked?: boolean;
-  @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 maxlength?: number;
   @Prop({ reflect: true }) public min?: number;
@@ -124,10 +124,10 @@ export class OdsInput {
     return;
   }
 
-  private renderButtonIcon(icon: ODS_ICON_NAME, callback: () => Promise<void>): FunctionalComponent {
+  private renderButtonIcon(icon: ODS_ICON_NAME, callback: () => Promise<void>, customClass: string = ''): FunctionalComponent {
     return (
       <button
-        class="ods-input__button"
+        class= { `ods-input__button ${customClass}` }
         disabled= { this.isDisabled }
         onClick={ callback }
         onKeyUp={ (event: KeyboardEvent): Promise<void> => handleKeySpace(event, this.isDisabled, callback) }>
@@ -171,11 +171,11 @@ export class OdsInput {
         }
 
         {
-          this.isPassword && !this.isLoading && this.renderButtonIcon(this.isMasked ? ODS_ICON_NAME.eyeClose : ODS_ICON_NAME.eyeOpen, this.toggleMask.bind(this))
+          this.isClearable && !this.isLoading && this.value && this.renderButtonIcon(ODS_ICON_NAME.cross, this.clear.bind(this), 'ods-input__button__clearable')
         }
 
         {
-          this.isClearable && !this.isLoading && this.renderButtonIcon(ODS_ICON_NAME.cross, this.clear.bind(this))
+          this.isPassword && !this.isLoading && this.renderButtonIcon(this.isMasked ? ODS_ICON_NAME.eyeClose : ODS_ICON_NAME.eyeOpen, this.toggleMask.bind(this))
         }
       </Host>
     );

diff --git a/packages/ods/src/components/input/src/index.html b/packages/ods/src/components/input/src/index.html
@@ -12,7 +12,7 @@
 
 <body>
   <p>Default</p>
-  <ods-input value="Default">
+  <ods-input value="Default" type="number">
   </ods-input>
 
   <p>Clearable</p>
@@ -36,7 +36,7 @@
   </ods-input>
 
   <p>Masked</p>
-  <ods-input is-masked>
+  <ods-input is-masked type="number">
   </ods-input>
 
   <p>Masked & Clearable</p>

diff --git a/packages/ods/src/components/input/tests/behaviour/ods-input.e2e.ts b/packages/ods/src/components/input/tests/behaviour/ods-input.e2e.ts
@@ -1,7 +1,7 @@
 import type { E2EElement, E2EPage } from '@stencil/core/testing';
 import { newE2EPage } from '@stencil/core/testing';
 
-describe('ods-input accessibility', () => {
+describe('ods-input behaviour', () => {
   let el: E2EElement;
   let page: E2EPage;
   let part: E2EElement;

diff --git a/packages/ods/src/components/input/tests/navigation/ods-input.e2e.ts b/packages/ods/src/components/input/tests/navigation/ods-input.e2e.ts
@@ -48,12 +48,22 @@ describe('ods-input accessibility', () => {
   });
 
   describe('isClearable', () => {
-    it('Button clearable should be focusable', async() => {
+    it('should not have clearable button without value', async() => {
       await setup('<ods-input is-clearable></ods-input>');
 
       await page.keyboard.press('Tab');
       expect(await odsInputFocusedElementTagName()).toBe('INPUT');
 
+      await page.keyboard.press('Tab');
+      expect(await odsInputFocusedElementTagName()).not.toBe('BUTTON');
+    });
+
+    it('Button clearable should be focusable', async() => {
+      await setup('<ods-input is-clearable value="value"></ods-input>');
+
+      await page.keyboard.press('Tab');
+      expect(await odsInputFocusedElementTagName()).toBe('INPUT');
+
       await page.keyboard.press('Tab');
       expect(await odsInputFocusedElementTagName()).toBe('BUTTON');
 
@@ -199,7 +209,7 @@ describe('ods-input accessibility', () => {
   });
 
   it('should have 2 button focusable with masked & clearable', async() => {
-    await setup('<ods-input is-masked is-clearable></ods-input>');
+    await setup('<ods-input is-masked is-clearable value="value"></ods-input>');
     await page.keyboard.press('Tab');
     expect(await odsInputFocusedElementTagName()).toBe('INPUT');
 

diff --git a/packages/ods/src/components/input/tests/rendering/ods-input.spec.ts b/packages/ods/src/components/input/tests/rendering/ods-input.spec.ts
@@ -351,7 +351,7 @@ describe('ods-input rendering', () => {
         expect(root?.getAttribute('type')).toBe(typeValue);
       });
 
-      it('should not be set by default with type "text"', async() => {
+      it('should be set by default with type "text"', async() => {
         await setup('<ods-input></ods-input>');
 
         expect(root?.getAttribute('type')).toBe(ODS_INPUT_TYPE.text);

diff --git a/packages/ods/src/style/_input.scss b/packages/ods/src/style/_input.scss
@@ -10,6 +10,7 @@
   width: 100%;
   height: 32px;
   line-height: 1rem;
+  color: theme.$ods-color-text;
   font-size: 1rem;
 
   &:focus {

diff --git a/packages/storybook/stories/components/input/input.stories.ts b/packages/storybook/stories/components/input/input.stories.ts
@@ -1,8 +1,9 @@
 import type { Meta, StoryObj } from '@storybook/web-components';
+import type { TemplateResult } from 'lit-html';
+import { ODS_INPUT_TYPE, ODS_INPUT_TYPES } from '@ovhcloud/ods-components';
 import { defineCustomElement } from '@ovhcloud/ods-components/dist/components/ods-input';
 import { html } from 'lit-html';
 import { CONTROL_CATEGORY, orderControls } from '../../control';
-import { ODS_INPUT_TYPE, ODS_INPUT_TYPES } from '@ovhcloud/ods-components';
 
 defineCustomElement();
 
@@ -13,27 +14,51 @@ const meta: Meta = {
 
 export default meta;
 
+function baseInput(arg: Record<string, string>): TemplateResult {
+  return arg.isMasked !== undefined
+    ? html`<ods-input class="my-input"
+      ariaLabel="${arg.ariaLabel}"
+      ariaLabelledby="${arg.ariaLabelledby}"
+      has-error="${arg.hasError}"
+      is-clearable="${arg.isClearable}"
+      is-disabled="${arg.isDisabled}"
+      is-loading="${arg.isLoading}"
+      is-masked="${arg.isMasked}"
+      is-readonly="${arg.isReadonly}"
+      is-required="${arg.isRequired}"
+      max="${arg.max}"
+      maxlength="${arg.maxlength}"
+      min="${arg.min}"
+      minlength="${arg.minlength}"
+      pattern="${arg.pattern}"
+      placeholder="${arg.placeholder}"
+      step="${arg.step}"
+      type="${arg.type}">
+      </ods-input>`
+    : html`<ods-input class="my-input"
+      ariaLabel="${arg.ariaLabel}"
+      ariaLabelledby="${arg.ariaLabelledby}"
+      has-error="${arg.hasError}"
+      is-clearable="${arg.isClearable}"
+      is-disabled="${arg.isDisabled}"
+      is-loading="${arg.isLoading}"
+      is-readonly="${arg.isReadonly}"
+      is-required="${arg.isRequired}"
+      max="${arg.max}"
+      maxlength="${arg.maxlength}"
+      min="${arg.min}"
+      minlength="${arg.minlength}"
+      pattern="${arg.pattern}"
+      placeholder="${arg.placeholder}"
+      step="${arg.step}"
+      type="${arg.type}">
+      </ods-input>`;
+}
+
 export const Demo: StoryObj = {
   render: (arg) => html`
-<ods-input class="my-input"
-  ariaLabel="${arg.ariaLabel}"
-  ariaLabelledby="${arg.ariaLabelledby}"
-  has-error="${arg.hasError}"
-  is-clearable="${arg.isClearable}"
-  is-disabled="${arg.isDisabled}"
-  is-loading="${arg.isLoading}"
-  is-masked="${arg.isMasked}"
-  is-readonly="${arg.isReadonly}"
-  is-required="${arg.isRequired}"
-  max="${arg.max}"
-  maxlength="${arg.maxlength}"
-  min="${arg.min}"
-  minlength="${arg.minlength}"
-  pattern="${arg.pattern}"
-  placeholder="${arg.placeholder}"
-  step="${arg.step}"
-  type="${arg.type}">
-</ods-input>
+  ${arg.isMasked}
+  ${baseInput(arg)}
 <style>
   .my-input::part(input) {
     ${arg.customCss}