8000 fix(input, textarea): clearOnInput ignores key modifiers (#28639) · ionic-team/ionic-framework@8f7d87c · GitHub
[go: up one dir, main page]
Skip to content

Commit 8f7d87c

Browse files
liamdebeasiliamdebeasi
authored
fix(input, textarea): clearOnInput ignores key modifiers (#28639)
Issue number: resolves #28633 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> In #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? <!-- Please describe the behavior or changes that are being added by this PR. --> - 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 <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Playwright bug report for a9f34a5: microsoft/playwright#28495
1 parent fc88613 commit 8f7d87c

File tree

4 files changed

+152
-12
lines changed

4 files changed

+152
-12
lines changed

‎core/src/components/input/input.tsx

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,15 +549,37 @@ export class Input implements ComponentInterface {
549549
if (!this.shouldClearOnEdit()) {
550550
return;
551551
}
552+
553+
/**
554+
* The following keys do not modify the
555+
* contents of the input. As a result, pressing
556+
* them should not edit the input.
557+
*
558+
* We can't check to see if the value of the input
559+
* was changed because we call checkClearOnEdit
560+
* in a keydown listener, and the key has not yet
561+
* been added to the input.
562+
*/
563+
const IGNORED_KEYS = ['Enter', 'Tab', 'Shift', 'Meta', 'Alt', 'Control'];
564+
const pressedIgnoredKey = IGNORED_KEYS.includes(ev.key);
565+
552566
/**
553567
* Clear the input if the control has not been previously cleared during focus.
554568
* Do not clear if the user hitting enter to submit a form.
555569
*/
556-
if (!this.didInputClearOnEdit && this.hasValue() && ev.key !== 'Enter' && ev.key !== 'Tab') {
570+
if (!this.didInputClearOnEdit && this.hasValue() && !pressedIgnoredKey) {
557571
this.value = '';
558572
this.emitInputChange(ev);
559573
}
560-
this.didInputClearOnEdit = true;
574+
575+
/**
576+
* Pressing an IGNORED_KEYS first and
577+
* then an allowed key will cause the input to not
578+
* be cleared.
579+
*/
580+
if (!pressedIgnoredKey) {
581+
this.didInputClearOnEdit = true;
582+
}
561583
}
562584

563585
private onCompositionStart = () => {

‎core/src/components/input/test/clear-on-edit/input.e2e.ts

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect } from '@playwright/test';
22
import { test, configs } from '@utils/test/playwright';
33

4+
const IGNORED_KEYS = ['Enter', 'Tab', 'Shift', 'Meta', 'Alt', 'Control'];
5+
46
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
57
test.describe(title('input: clearOnEdit'), () => {
68
test('should clear when typed into', async ({ page }) => {
@@ -16,28 +18,57 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
1618
await expect(input).toHaveJSProperty('value', 'h');
1719
});
1820

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

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

25-
await page.keyboard.press('Enter');
26-
await page.waitForChanges();
26+
await input.click();
27+
F438 await input.type('hello world');
2728

28-
await expect(input).toHaveJSProperty('value', 'abc');
29+
await expect(input).toHaveJSProperty('value', 'hello world');
30+
});
31+
32+
IGNORED_KEYS.forEach((ignoredKey: string) => {
33+
test(`should not clear when ${ignoredKey} is pressed`, async ({ page, skip }) => {
34+
skip.browser(
35+
(browserName: string) => browserName === 'firefox' && ignoredKey === 'Meta',
36+
'Firefox incorrectly adds "OS" to the input when pressing the Meta key on Linux'
37+
);
38+
await page.setContent(`<ion-input value="abc" clear-on-edit="true" aria-label="input"></ion-input>`, config);
39+
40+
const input = page.locator('ion-input');
41+
await input.locator('input').focus();
42+
43+
await page.keyboard.press(ignoredKey);
44+
await page.waitForChanges();
45+
46+
await expect(input).toHaveJSProperty('value', 'abc');
47+
});
2948
});
3049

31-
test('should not clear when tab is pressed', async ({ page }) => {
50+
test('should clear after when pressing valid key after pressing ignored key', async ({ page }) => {
51+
test.info().annotations.push({
52+
type: 'issue',
53+
description: 'https://github.com/ionic-team/ionic-framework/issues/28633',
54+
});
55+
3256
await page.setContent(`<ion-input value="abc" clear-on-edit="true" aria-label="input"></ion-input>`, config);
3357

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

37-
await page.keyboard.press('Tab');
61+
// ignored
62+
await page.keyboard.press('Shift');
3863
await page.waitForChanges();
3964

4065
await expect(input).toHaveJSProperty('value', 'abc');
66+
67+
// allowed
68+
await page.keyboard.press('a');
69+
await page.waitForChanges();
70+
71+
await expect(input).toHaveJSProperty('value', 'a');
4172
});
4273
});
4374
});

‎core/src/components/textarea/test/clear-on-edit/textarea.e2e.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect } from '@playwright/test';
22
import { test, configs } from '@utils/test/playwright';
33

4+
const IGNORED_KEYS = ['Tab', 'Shift', 'Meta', 'Alt', 'Control'];
5+
46
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
57
test.describe(title('textarea: clearOnEdit'), () => {
68
test('should clear when typed into', async ({ page }) => {
@@ -33,5 +35,64 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
3335

3436
await expect(textarea).toHaveJSProperty('value', 'abc');
3537
});
38+
39+
test('should not clear the textarea if it does not have an initial value when typing', async ({ page }) => {
40+
await page.setContent(`<ion-textarea label="textarea" value="" clear-on-edit="true"></ion-textarea>`, config);
41+
42+
const textarea = page.locator('ion-textarea');
43+
44+
await textarea.click();
45+
await textarea.type('hello world');
46+
47+
await expect(textarea).toHaveJSProperty('value', 'hello world');
48+
});
49+
50+
IGNORED_KEYS.forEach((ignoredKey: string) => {
51+
test(`should not clear when ${ignoredKey} is pressed`, async ({ page, skip }) => {
52+
skip.browser(
53+
(browserName: string) => browserName === 'firefox' && ignoredKey === 'Meta',
54+
'Firefox incorrectly adds "OS" to the textarea when pressing the Meta key on Linux'
55+
);
56+
await page.setContent(
57+
`<ion-textarea value="abc" clear-on-edit="true" aria-label="textarea"></ion-textarea>`,
58+
config
59+
);
60+
61+
const textarea = page.locator('ion-textarea');
62+
await textarea.locator('textarea').focus();
63+
64+
await page.keyboard.press(ignoredKey);
65+
await page.waitForChanges();
66+
67+
await expect(textarea).toHaveJSProperty('value', 'abc');
68+
});
69+
});
70+
71+
test('should clear after when pressing valid key after pressing ignored key', async ({ page }) => {
72+
test.info().annotations.push({
73+
type: 'issue',
74+
description: 'https://github.com/ionic-team/ionic-framework/issues/28633',
75+
});
76+
77+
await page.setContent(
78+
`<ion-textarea value="abc" clear-on-edit="true" aria-label="textarea"></ion-textarea>`,
79+
config
80+
);
81+
82+
const textarea = page.locator('ion-textarea');
83+
await textarea.locator('textarea').focus();
84+
85+
// ignored
86+
await page.keyboard.press('Shift');
87+
await page.waitForChanges();
88+
89+
await expect(textarea).toHaveJSProperty('value', 'abc');
90+
91+
// allowed
92+
await page.keyboard.press('a');
93+
await page.waitForChanges();
94+
95+
await expect(textarea).toHaveJSProperty('value', 'a');
96+
});
3697
});
3798
});

‎core/src/components/textarea/textarea.tsx

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,15 +459,41 @@ export class Textarea implements ComponentInterface {
459459
if (!this.clearOnEdit) {
460460
return;
461461
}
462+
463+
/**
464+
* The following keys do not modify the
465+
* contents of the input. As a result, pressing
466+
* them should not edit the textarea.
467+
*
468+
* We can't check to see if the value of the textarea
469+
* was changed because we call checkClearOnEdit
470+
* in a keydown listener, and the key has not yet
471+
* been added to the textarea.
472+
*
473+
* Unlike ion-input, the "Enter" key does modify the
474+
* textarea by adding a new line, so "Enter" is not
475+
* included in the IGNORED_KEYS array.
476+
*/
477+
const IGNORED_KEYS = ['Tab', 'Shift', 'Meta', 'Alt', 'Control'];
478+
const pressedIgnoredKey = IGNORED_KEYS.includes(ev.key);
479+
462480
/**
463481
* Clear the textarea if the control has not been previously cleared
464482
* during focus.
465483
*/
466-
if (!this.didTextareaClearOnEdit && this.hasValue() && ev.key !== 'Tab') {
484+
if (!this.didTextareaClearOnEdit && this.hasValue() && !pressedIgnoredKey) {
467485
this.value = '';
468486
this.emitInputChange(ev);
469487
}
470-
this.didTextareaClearOnEdit = true;
488+
489+
/**
490+
* Pressing an IGNORED_KEYS first and
491+
* then an allowed key will cause the input to not
492+
* be cleared.
493+
*/
494+
if (!pressedIgnoredKey) {
495+
this.didTextareaClearOnEdit = true;
496+
}
471497
}
472498

473499
private focusChange() {

0 commit comments

Comments
 (0)
0