From 8f7d87c6803b1600a3ca21785df0e9bac49f74a3 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Mon, 11 Dec 2023 15:35:25 -0500 Subject: [PATCH] fix(input, textarea): clearOnInput ignores key modifiers (#28639) Issue number: resolves #28633 --------- ## What is the current behavior? In https://github.com/ionic-team/ionic-framework/pull/28005 I introduced a fix that causes "clearOnEdit" to not clear input/textarea when the Tab key was pressed. However, there were several edge cases I missed. For instance, pressing the "shift" key and then the "tab" key would still clear the input because "shift" was not explicitly excluded. ## What is the new behavior? - Input and textarea now explicitly ignores modifier keys that do not change the value of the input - `didInputClearOnEdit` is not set to `true` when an ignored key is pressed. Otherwise, pressing an ignored key and then an accepted key would not cause the input to be cleared. For example, pressing "shift", releasing "shift", then pressing "A" should cause the input to still get cleared. - Added test coverage ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Playwright bug report for https://github.com/ionic-team/ionic-framework/pull/28639/commits/a9f34a54f1bf5b7dfdc1401f5a339f55c4b9093b: https://github.com/microsoft/playwright/issues/28495 --- core/src/components/input/input.tsx | 26 +++++++- .../input/test/clear-on-edit/input.e2e.ts | 47 +++++++++++--- .../test/clear-on-edit/textarea.e2e.ts | 61 +++++++++++++++++++ core/src/components/textarea/textarea.tsx | 30 ++++++++- 4 files changed, 152 insertions(+), 12 deletions(-) diff --git a/core/src/components/input/input.tsx b/core/src/components/input/input.tsx index e8cc049274..d28cfd385c 100644 --- a/core/src/components/input/input.tsx +++ b/core/src/components/input/input.tsx @@ -549,15 +549,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 = () => { diff --git a/core/src/components/input/test/clear-on-edit/input.e2e.ts b/core/src/components/input/test/clear-on-edit/input.e2e.ts index e668690278..ecfbecb3c1 100644 --- a/core/src/components/input/test/clear-on-edit/input.e2e.ts +++ b/core/src/components/input/test/clear-on-edit/input.e2e.ts @@ -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']; + configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('input: clearOnEdit'), () => { test('should clear when typed into', async ({ page }) => { @@ -16,28 +18,57 @@ 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(``, config); + test('should not clear the input if it does not have an initial value when typing', async ({ page }) => { + await page.setContent(``, 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'); }); - test('should not clear when tab is pressed', async ({ page }) => { + IGNORED_KEYS.forEach((ignoredKey: string) => { + test(`should not clear when ${ignoredKey} is pressed`, async ({ page, skip }) => { + skip.browser( + (browserName: string) => browserName === 'firefox' && ignoredKey === 'Meta', + 'Firefox incorrectly adds "OS" to the input when pressing the Meta key on Linux' + ); + await page.setContent(``, 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 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(``, 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'); }); }); }); diff --git a/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts b/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts index f085f46ee8..898b63a2d6 100644 --- a/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts +++ b/core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts @@ -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 }) => { @@ -33,5 +35,64 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => 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(``, 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, skip }) => { + skip.browser( + (browserName: string) => browserName === 'firefox' && ignoredKey === 'Meta', + 'Firefox incorrectly adds "OS" to the textarea when pressing the Meta key on Linux' + ); + await page.setContent( + ``, + 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'); + }); + }); + + 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( + ``, + 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'); + }); }); }); diff --git a/core/src/components/textarea/textarea.tsx b/core/src/components/textarea/textarea.tsx index f971c8ee65..83557d3254 100644 --- a/core/src/components/textarea/textarea.tsx +++ b/core/src/components/textarea/textarea.tsx @@ -459,15 +459,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() {