Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(input, textarea): clearOnInput ignores key modifiers #28639

Merged
merged 11 commits into from
Dec 11, 2023
26 changes: 24 additions & 2 deletions core/src/components/input/input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -545,15 +545,37 @@ export class Input implements ComponentInterface {
if (!this.shouldClearOnEdit()) {
return;
}

/**
* The following keys do not modify the
* contents of the input. As a result, pressing
* them should not edit the input.
*
* We can't check to see if the value of the input
* was changed because we call checkClearOnEdit
* in a keydown listener, and the key has not yet
* been added to the input.
*/
const IGNORED_KEYS = ['Enter', 'Tab', 'Shift', 'Meta', 'Alt', 'Control'];
const pressedIgnoredKey = IGNORED_KEYS.includes(ev.key);

/**
* Clear the input if the control has not been previously cleared during focus.
* Do not clear if the user hitting enter to submit a form.
*/
if (!this.didInputClearOnEdit && this.hasValue() && ev.key !== 'Enter' && ev.key !== 'Tab') {
if (!this.didInputClearOnEdit && this.hasValue() && !pressedIgnoredKey) {
this.value = '';
this.emitInputChange(ev);
}
this.didInputClearOnEdit = true;

/**
* Pressing an IGNORED_KEYS first and
* then an allowed key will cause the input to not
* be cleared.
*/
if (!pressedIgnoredKey) {
this.didInputClearOnEdit = true;
}
}

private onCompositionStart = () => {
Expand Down
43 changes: 35 additions & 8 deletions core/src/components/input/test/clear-on-edit/input.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from '@playwright/test';
import { test, configs } from '@utils/test/playwright';

const IGNORED_KEYS = ['Enter', 'Tab', 'Shift', 'Meta', 'Alt', 'Control'];
liamdebeasi marked this conversation as resolved.
Show resolved Hide resolved

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('input: clearOnEdit'), () => {
test('should clear when typed into', async ({ page }) => {
Expand All @@ -16,28 +18,53 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
await expect(input).toHaveJSProperty('value', 'h');
});

test('should not clear when enter is pressed', async ({ page }) => {
await page.setContent(`<ion-input value="abc" clear-on-edit="true" aria-label="input"></ion-input>`, config);
test('should not clear the input if it does not have an initial value when typing', async ({ page }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new test (and the same textarea test) was copied over from the legacy directory. I accidentally regressed a behavior that only the legacy tests caught. Since the behavior is still relevant for the modern form control syntax, I decided to copy them over so we don't lose test coverage when we remove the legacy form syntax.

await page.setContent(`<ion-input label="input" value="" clear-on-edit="true"></ion-input>`, config);

const input = page.locator('ion-input');
await input.locator('input').focus();

await page.keyboard.press('Enter');
await page.waitForChanges();
await input.click();
await input.type('hello world');

await expect(input).toHaveJSProperty('value', 'abc');
await expect(input).toHaveJSProperty('value', 'hello world');
});

IGNORED_KEYS.forEach((ignoredKey: string) => {
test(`should not clear when ${ignoredKey} is pressed`, async ({ page }) => {
await page.setContent(`<ion-input value="abc" clear-on-edit="true" aria-label="input"></ion-input>`, config);

const input = page.locator('ion-input');
await input.locator('input').focus();

await page.keyboard.press(ignoredKey);
await page.waitForChanges();

await expect(input).toHaveJSProperty('value', 'abc');
});
});

test('should not clear when tab is pressed', async ({ page }) => {
test('should clear after when pressing valid key after pressing ignored key', async ({ page }) => {
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/28633',
});

await page.setContent(`<ion-input value="abc" clear-on-edit="true" aria-label="input"></ion-input>`, config);

const input = page.locator('ion-input');
await input.locator('input').focus();

await page.keyboard.press('Tab');
// ignored
await page.keyboard.press('Shift');
await page.waitForChanges();

await expect(input).toHaveJSProperty('value', 'abc');

// allowed
await page.keyboard.press('a');
await page.waitForChanges();

await expect(input).toHaveJSProperty('value', 'a');
});
});
});
57 changes: 57 additions & 0 deletions core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from '@playwright/test';
import { test, configs } from '@utils/test/playwright';

const IGNORED_KEYS = ['Tab', 'Shift', 'Meta', 'Alt', 'Control'];

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('textarea: clearOnEdit'), () => {
test('should clear when typed into', async ({ page }) => {
Expand Down Expand Up @@ -33,5 +35,60 @@

await expect(textarea).toHaveJSProperty('value', 'abc');
});

test('should not clear the textarea if it does not have an initial value when typing', async ({ page }) => {
await page.setContent(`<ion-textarea label="textarea" value="" clear-on-edit="true"></ion-textarea>`, config);

const textarea = page.locator('ion-textarea');

await textarea.click();
await textarea.type('hello world');

await expect(textarea).toHaveJSProperty('value', 'hello world');
});

IGNORED_KEYS.forEach((ignoredKey: string) => {
test(`should not clear when ${ignoredKey} is pressed`, async ({ page }) => {
await page.setContent(
`<ion-textarea value="abc" clear-on-edit="true" aria-label="textarea"></ion-textarea>`,
config
);

const textarea = page.locator('ion-textarea');
await textarea.locator('textarea').focus();

await page.keyboard.press(ignoredKey);
await page.waitForChanges();

await expect(textarea).toHaveJSProperty('value', 'abc');

Check failure on line 63 in core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts

View workflow job for this annotation

GitHub Actions / test-core-screenshot (6, 20)

[Mobile Firefox] › src/components/textarea/test/clear-on-edit/textarea.e2e.ts:51:11 › textarea: clearOnEdit - ios/ltr › should not clear when Meta is pressed

1) [Mobile Firefox] › src/components/textarea/test/clear-on-edit/textarea.e2e.ts:51:11 › textarea: clearOnEdit - ios/ltr › should not clear when Meta is pressed Error: Timed out 5000ms waiting for expect(locator).toHaveJSProperty(expected) Locator: locator('ion-textarea') Expected: "abc" Received: "OS" Call log: - expect.toHaveJSProperty with timeout 5000ms - waiting for locator('ion-textarea') - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" 61 | await page.waitForChanges(); 62 | > 63 | await expect(textarea).toHaveJSProperty('value', 'abc'); | ^ 64 | }); 65 | }); 66 | at /home/runner/work/ionic-framework/ionic-framework/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts:63:32

Check failure on line 63 in core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts

View workflow job for this annotation

GitHub Actions / test-core-screenshot (6, 20)

[Mobile Firefox] › src/components/textarea/test/clear-on-edit/textarea.e2e.ts:51:11 › textarea: clearOnEdit - ios/ltr › should not clear when Meta is pressed

1) [Mobile Firefox] › src/components/textarea/test/clear-on-edit/textarea.e2e.ts:51:11 › textarea: clearOnEdit - ios/ltr › should not clear when Meta is pressed Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(locator).toHaveJSProperty(expected) Locator: locator('ion-textarea') Expected: "abc" Received: "OS" Call log: - expect.toHaveJSProperty with timeout 5000ms - waiting for locator('ion-textarea') - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" 61 | await page.waitForChanges(); 62 | > 63 | await expect(textarea).toHaveJSProperty('value', 'abc'); | ^ 64 | }); 65 | }); 66 | at /home/runner/work/ionic-framework/ionic-framework/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts:63:32

Check failure on line 63 in core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts

View workflow job for this annotation

GitHub Actions / test-core-screenshot (6, 20)

[Mobile Firefox] › src/components/textarea/test/clear-on-edit/textarea.e2e.ts:51:11 › textarea: clearOnEdit - ios/ltr › should not clear when Meta is pressed

1) [Mobile Firefox] › src/components/textarea/test/clear-on-edit/textarea.e2e.ts:51:11 › textarea: clearOnEdit - ios/ltr › should not clear when Meta is pressed Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(locator).toHaveJSProperty(expected) Locator: locator('ion-textarea') Expected: "abc" Received: "OS" Call log: - expect.toHaveJSProperty with timeout 5000ms - waiting for locator('ion-textarea') - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" - locator resolved to <ion-textarea value="abc" clear-on-edit="true" class="sc-ion-t…>…</ion-textarea> - unexpected value "OS" 61 | await page.waitForChanges(); 62 | > 63 | await expect(textarea).toHaveJSProperty('value', 'abc'); | ^ 64 | }); 65 | }); 66 | at /home/runner/work/ionic-framework/ionic-framework/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts:63:32
});
});

test('should clear after when pressing valid key after pressing ignored key', async ({ page }) => {
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/28633',
});

await page.setContent(
`<ion-textarea value="abc" clear-on-edit="true" aria-label="textarea"></ion-textarea>`,
config
);

const textarea = page.locator('ion-textarea');
await textarea.locator('textarea').focus();

// ignored
await page.keyboard.press('Shift');
await page.waitForChanges();

await expect(textarea).toHaveJSProperty('value', 'abc');

// allowed
await page.keyboard.press('a');
await page.waitForChanges();

await expect(textarea).toHaveJSProperty('value', 'a');
});
});
});
30 changes: 28 additions & 2 deletions core/src/components/textarea/textarea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -455,15 +455,41 @@ export class Textarea implements ComponentInterface {
if (!this.clearOnEdit) {
return;
}

/**
* The following keys do not modify the
* contents of the input. As a result, pressing
* them should not edit the textarea.
*
* We can't check to see if the value of the textarea
* was changed because we call checkClearOnEdit
* in a keydown listener, and the key has not yet
* been added to the textarea.
*
* Unlike ion-input, the "Enter" key does modify the
* textarea by adding a new line, so "Enter" is not
* included in the IGNORED_KEYS array.
*/
const IGNORED_KEYS = ['Tab', 'Shift', 'Meta', 'Alt', 'Control'];
const pressedIgnoredKey = IGNORED_KEYS.includes(ev.key);

/**
* Clear the textarea if the control has not been previously cleared
* during focus.
*/
if (!this.didTextareaClearOnEdit && this.hasValue() && ev.key !== 'Tab') {
if (!this.didTextareaClearOnEdit && this.hasValue() && !pressedIgnoredKey) {
this.value = '';
this.emitInputChange(ev);
}
this.didTextareaClearOnEdit = true;

/**
* Pressing an IGNORED_KEYS first and
* then an allowed key will cause the input to not
* be cleared.
*/
if (!pressedIgnoredKey) {
this.didTextareaClearOnEdit = true;
}
}

private focusChange() {
Expand Down
Loading