-
Notifications
You must be signed in to change notification settings - Fork 179
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 issues related to selecting all text #11099
Conversation
Can you provide a bit more context in the PR description please. |
Size Change: -572 B (0%) Total Size: 2.59 MB
ℹ️ View Unchanged
|
Plugin builds for 2d3416a are ready 🛎️!
|
I've updated the default title and added some details about the solution, more details about the issue itself can be found in the linked tickets. |
Can we add Karma tests for this change? |
It's tricky, I believe you saw that: puppeteer/puppeteer#1313 // Testing this on the BROKEN version
await fixture.events.mouse.clickOn(frame, 30, 5); // enter the edit mode by clicking
await fixture.events.keyboard.shortcut('mod+a'); // doesn't work on macOS, maybe will work on Ubuntu
document.execCommand("selectAll"); // selects all, but it's not the same as MOD+A, so we can't use it
await fixture.events.keyboard.type('BAR');
// result: BAR, expected: Lorem...BAR...ipsum |
That could work, yeah. Better than no tests at all. |
@swissspidy We now have a test for it, PTAL at those |
I think it's OK like that 👍 |
Summary
There are issues with selecting all text that were reported in #11034 and #11061.
There might be a different/better solution (because CMD+A generally works, it's more a problem with selection/focus), but this one is easy and harmless and most importantly - fixes all the issues with selecting all text.
I tell CMD+A to explicitly select all text in the text element.
To-do
User-facing changes
Testing Instructions
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
Does this PR change what data or activity we track or use?
Does this PR have a legal-related impact?
Checklist
Type: XYZ
label to the PRFixes #11034
Fixes #11061