Skip to content

Commit

Permalink
fix(input): review
Browse files Browse the repository at this point in the history
  • Loading branch information
aesteves60 authored and dpellier committed Jul 29, 2024
1 parent 0a62bc7 commit 1e4b54a
Show file tree
Hide file tree
Showing 12 changed files with 201 additions and 205 deletions.
2 changes: 1 addition & 1 deletion packages/ods/src/components/input/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"clean": "rimraf .stencil coverage dist docs-api www",
"doc": "typedoc --pretty --plugin ../../../scripts/typedoc-plugin-decorator.js && node ../../../scripts/generate-typedoc-md.js",
"lint:scss": "stylelint 'src/components/**/*.scss'",
"lint:ts": "eslint 'src/**/*.{js,ts,tsx}'",
"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:ci": "tsc --noEmit && stencil test --e2e --ci --config stencil.config.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
position: relative;
}

$ods-input__button-right: 4px;

.ods-input {
&__button {
display: inline-flex;
position: absolute;
top: 0;
right: 4px;
right: $ods-input__button-right;
border: none;
background: none;
color: theme.$ods-color-primary-500;
Expand Down Expand Up @@ -41,7 +43,7 @@
}

&--disabled {
background: theme.$ods-color-neutral-100;
background-color: theme.$ods-color-neutral-100;
cursor: not-allowed;
user-select: none;

Expand All @@ -58,12 +60,12 @@
&__spinner {
position: absolute;
top: 0;
right: 4px;
right: $ods-input__button-right;
padding: 0 4px;

&::part(spinner) {
width: 16px;
height: 16px;
width: 1rem;
height: 1rem;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AttachInternals, Component, Element, Event, type EventEmitter, type FunctionalComponent, Host, Method, Prop, State, Watch, h } from '@stencil/core';
import { ODS_ICON_NAME } from '../../../../icon/src';
import { ODS_INPUT_TYPE, type OdsInputType } from '../../constants/input-type';
import { handleKeySpace, isPassword } from '../../controller/ods-input';
import { handleKeySpace, isPassword, setFormValue } from '../../controller/ods-input';
import { type OdsInputEventValueChange } from '../../interfaces/events';

@Component({
Expand All @@ -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;
@Prop({ reflect: true }) public isClearable?: boolean = false;
@Prop({ reflect: true }) public isDisabled: boolean = false;
@Prop({ reflect: true }) public isLoading?: boolean;
@Prop({ reflect: true }) public isLoading?: boolean = false;
@Prop({ mutable: true, reflect: true }) public isMasked?: boolean;
@Prop({ reflect: true }) public isReadonly?: boolean;
@Prop({ reflect: true }) public isRequired?: boolean;
@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;
Expand Down Expand Up @@ -94,7 +94,7 @@ export class OdsInput {

@Watch('value')
onValueChange(value: string | number, oldValue?: string | number): void {
this.internals?.setFormValue?.(this.value?.toString() ?? '');
setFormValue(this.internals, this.value);
this.onErrorChange();
this.odsValueChange.emit({
name: this.name,
Expand All @@ -109,7 +109,7 @@ export class OdsInput {
if (!this.value) {
this.value = this.defaultValue;
}
this.internals?.setFormValue?.(this.value?.toString() ?? '');
setFormValue(this.internals, this.value);
}

async formResetCallback(): Promise<void> {
Expand All @@ -129,7 +129,7 @@ export class OdsInput {
<button
class="ods-input__button"
disabled= { this.isDisabled }
onClick={ (): Promise<void> => callback() }
onClick={ callback }
onKeyUp={ (event: KeyboardEvent): Promise<void> => handleKeySpace(event, this.isDisabled, callback) }>
<ods-icon name={ icon }>
</ods-icon>
Expand Down
44 changes: 5 additions & 39 deletions packages/ods/src/components/input/src/controller/ods-input.ts
Original file line number Diff line number Diff line change
@@ -1,54 +1,20 @@
// import { EventEmitter } from "@stencil/core";

// function handleClear(eventEmitter: EventEmitter<void>, isDisabled: boolean): undefined | void {
// if (isDisabled) {
// return;
// }
// eventEmitter.emit();
// return undefined;
// }

async function handleKeySpace(event: KeyboardEvent, isDisabled: boolean, callback: () => Promise<void>): Promise<void> {
event?.preventDefault?.();
event.preventDefault();
if(event.key === ' ' && !isDisabled) {
await callback();
}
}

// function handleOnInput(eventEmitter: EventEmitter<{ value?: string | number}>, isDisabled: boolean, internals: ElementInternals, inputEl?: HTMLInputElement): { value?: string | number, error: boolean } {
// if (isDisabled) {
// return;
// }
// inputEl?.value && internals.setFormValue(inputEl.value.toString());
// eventEmitter.emit({ value: inputEl?.value });
// return { value: inputEl?.value, error: !inputEl?.validity.valid ?? false }
// }

// function handleReset(eventEmitter: EventEmitter<void>, isDisabled: boolean, defaultValue?: string | number): string | number | undefined {
// if (isDisabled) {
// return;
// }
// eventEmitter.emit();
// return defaultValue ?? undefined;
// }

// function handleToggleMask(eventEmitter: EventEmitter<void>, isDisabled: boolean, masked?: boolean): boolean {
// if (isDisabled) {
// return;
// }
// eventEmitter.emit();
// return !masked;
// }
function setFormValue(internals: ElementInternals, value?: number | string): void {
internals?.setFormValue?.(value?.toString() ?? '');
}

function isPassword(isMasked?: boolean): boolean {
return isMasked !== undefined;
}

export {
// handleClear,
handleKeySpace,
// handleOnInput,
// handleToggleMask,
// handleReset,
isPassword,
setFormValue,
};
114 changes: 114 additions & 0 deletions packages/ods/src/components/input/tests/behaviour/ods-input.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import type { E2EElement, E2EPage } from '@stencil/core/testing';
import { newE2EPage } from '@stencil/core/testing';

describe('ods-input accessibility', () => {
let el: E2EElement;
let page: E2EPage;
let part: E2EElement;

async function setup(content: string): Promise<void> {
page = await newE2EPage();

await page.setContent(content);
await page.evaluate(() => document.body.style.setProperty('margin', '0px'));

el = await page.find('ods-input');
part = await page.find('ods-input >>> [part="input"]');
await page.waitForChanges();
}

beforeEach(jest.clearAllMocks);

describe('method:clear', () => {
it('should receive odsClear event', async() => {
await setup('<ods-input value="value"></ods-input>');
const odsClearSpy = await page.spyOnEvent('odsClear');
await el.callMethod('clear');
await page.waitForChanges();
expect(await el.getProperty('value')).toBeNull();
expect(odsClearSpy).toHaveReceivedEventTimes(1);
});

it('should do nothing because of disabled', async() => {
const value = 'value';
await setup(`<ods-input is-disabled value="${value}"></ods-input>`);
const odsClearSpy = await page.spyOnEvent('odsClear');
await el.callMethod('clear');
await page.waitForChanges();
expect(await el.getProperty('value')).toBe(value);
expect(odsClearSpy).not.toHaveReceivedEvent();
});
});

describe('method:reset', () => {
it('should receive odsReset event', async() => {
const defaultValue = 'defaultValue';
await setup(`<ods-input value="value" default-value="${defaultValue}"></ods-input>`);
const odsResetSpy = await page.spyOnEvent('odsReset');
await el.callMethod('reset');
await page.waitForChanges();
expect(await el.getProperty('value')).toBe(defaultValue);
expect(odsResetSpy).toHaveReceivedEventTimes(1);
});

it('should do nothing because of disabled', async() => {
const value = 'value';
await setup(`<ods-input is-disabled value="${value}" default-value="defaultValue"></ods-input>`);
const odsResetSpy = await page.spyOnEvent('odsReset');
await el.callMethod('reset');
await page.waitForChanges();
expect(await el.getProperty('value')).toBe(value);
expect(odsResetSpy).not.toHaveReceivedEvent();
});
});

describe('method:toggleMask', () => {
it('should toggle mask', async() => {
await setup('<ods-input is-masked></ods-input>');
const odsToggleMaskSpy = await page.spyOnEvent('odsToggleMask');
await el.callMethod('toggleMask');
await page.waitForChanges();
expect(await el.getProperty('isMasked')).toBe(false);

await el.callMethod('toggleMask');
await page.waitForChanges();
expect(await el.getProperty('isMasked')).toBe(true);
expect(odsToggleMaskSpy).toHaveReceivedEventTimes(2);
});

it('should do nothing because of disabled', async() => {
await setup('<ods-input is-masked is-disabled></ods-input>');
const odsToggleMaskSpy = await page.spyOnEvent('odsToggleMask');
await el.callMethod('toggleMask');
await page.waitForChanges();

expect(await el.getProperty('isMasked')).toBe(true);
expect(odsToggleMaskSpy).not.toHaveReceivedEvent();
});
});

describe('event:odsValueChange', () => {
it('should receive odsValueChange event', async() => {
const typeValue = 'some text';
await setup('<ods-input></ods-input>');
const odsValueChangeSpy = await page.spyOnEvent('odsValueChange');

await part.type(typeValue);
await page.waitForChanges();

expect(await el.getProperty('value')).toBe(typeValue);
expect(odsValueChangeSpy).toHaveReceivedEventTimes(typeValue.length);
});

it('should do nothing because of disabled', async() => {
await setup('<ods-input is-disabled></ods-input>');
const odsValueChangeSpy = await page.spyOnEvent('odsValueChange');

await part.type('some text');
await page.waitForChanges();

expect(await el.getProperty('value')).toBe(undefined);
expect(odsValueChangeSpy).not.toHaveReceivedEvent();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,29 @@ describe('ods-input controller', () => {
const callback = jest.fn();
const event = {
key: ' ',
preventDefault: jest.fn(),
} as unknown as KeyboardEvent;
handleKeySpace(event, true, callback)
handleKeySpace(event, true, callback);
expect(callback).not.toHaveBeenCalled();
});

it('should call callback if space', async() => {
const callback = jest.fn();
const event = {
key: ' ',
preventDefault: jest.fn(),
} as unknown as KeyboardEvent;
handleKeySpace(event, false, callback)
handleKeySpace(event, false, callback);
expect(callback).toHaveBeenCalledTimes(1);
});

it('should do nothing if not space', async() => {
const callback = jest.fn();
const event = {
key: 'Enter',
preventDefault: jest.fn(),
} as unknown as KeyboardEvent;
handleKeySpace(event, false, callback)
handleKeySpace(event, false, callback);
expect(callback).not.toHaveBeenCalled();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('ods-input accessibility', () => {

el = await page.find('ods-input');
input = await page.find('ods-input >>> input');
button = await page.find('ods-input >>> .ods-input__button')
button = await page.find('ods-input >>> .ods-input__button');
}

beforeEach(jest.clearAllMocks);
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('ods-input accessibility', () => {
const odsClearSpy = await page.spyOnEvent('odsClear');
expect(await el.getProperty('value')).toBe('value');

await button.click()
await button.click();
await page.waitForChanges();

expect(await el.getProperty('value')).toBeNull();
Expand All @@ -109,11 +109,14 @@ describe('ods-input accessibility', () => {
await page.keyboard.press('Tab');
await page.keyboard.press('Space');
await page.waitForChanges();

expect(await el.getProperty('value')).toBe('value');

await page.keyboard.press('Enter');
await page.waitForChanges();
expect(await el.getProperty('value')).toBe('value');

await button.click();
await page.waitForChanges();
expect(await el.getProperty('value')).toBe('value');
expect(odsClearSpy).not.toHaveReceivedEvent();
});
Expand Down Expand Up @@ -154,7 +157,7 @@ describe('ods-input accessibility', () => {

await page.keyboard.press('Tab');
await page.keyboard.press('Tab');
await page.keyboard.press(' ');
await page.keyboard.press('Space');
await page.waitForChanges();

expect(await input.getProperty('type')).toBe('text');
Expand All @@ -166,7 +169,7 @@ describe('ods-input accessibility', () => {
const odsToggleMaskSpy = await page.spyOnEvent('odsToggleMask');
expect(await input.getProperty('type')).toBe('password');

await button.click()
await button.click();
await page.waitForChanges();

expect(await input.getProperty('type')).toBe('text');
Expand All @@ -182,13 +185,14 @@ describe('ods-input accessibility', () => {
await page.keyboard.press('Tab');
await page.keyboard.press('Space');
await page.waitForChanges();

expect(await input.getProperty('type')).toBe('password');

await page.keyboard.press('Enter');
await page.waitForChanges();
await button.click();
expect(await input.getProperty('type')).toBe('password');

await button.click();
await page.waitForChanges();
expect(await input.getProperty('type')).toBe('password');
expect(odsToggleMaskSpy).not.toHaveReceivedEvent();
});
Expand Down
Loading

0 comments on commit 1e4b54a

Please sign in to comment.