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
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Dec 5, 2023

Issue number: resolves #28633


What is the current behavior?

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?

  • 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
  • No

Other information

Playwright bug report for a9f34a5: microsoft/playwright#28495

@liamdebeasi liamdebeasi changed the title Fw 5692 fix(input, textarea): clearOnInput ignores key modifiers Dec 5, 2023
@github-actions github-actions bot added the package: core @ionic/core package label Dec 5, 2023
@liamdebeasi
Copy link
Contributor Author

Reviewers: This task was completed outside of my sprint work, so sprint work-related PRs take higher priority.

@@ -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.

@liamdebeasi liamdebeasi marked this pull request as ready for review December 5, 2023 16:35
@liamdebeasi liamdebeasi requested a review from a team as a code owner December 5, 2023 16:35
@liamdebeasi liamdebeasi requested review from thetaPC and removed request for a team December 5, 2023 16:35
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

Works as intended! Just one comment.

Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@liamdebeasi liamdebeasi added this pull request to the merge queue Dec 11, 2023
Merged via the queue into main with commit 8f7d87c Dec 11, 2023
46 checks passed
@liamdebeasi liamdebeasi deleted the FW-5692 branch December 11, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: IonInput type="password" clears input field on modifier key press
2 participants