-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Restore native backspace behaviour #11627
Conversation
There's one failing e2e test: if you select a whole paragraph, it seems TinyMCE is doing some weird stuff. |
@@ -252,9 +252,11 @@ export default class TinyMCE extends Component { | |||
|
|||
onKeyDown( event ) { | |||
const { keyCode } = event; | |||
const { startContainer, startOffset, endContainer, endOffset } = getSelection().getRangeAt( 0 ); | |||
const isCollapsed = startContainer === endContainer && startOffset === endOffset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getSelection().isCollapsed
is not accurate, should prob be commented.
const start = getSelectionStart( value ); | ||
const end = getSelectionEnd( value ); | ||
|
||
// Always handle uncollapsed selections ourselves. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use more comment sorry.
Thanks! Could you also add an e2e test for word wise deletion alt+backspace? Thanks so much! |
Sure :) |
await page.keyboard.type( 'delta' ); | ||
const blockText = await page.evaluate( () => document.activeElement.textContent ); | ||
expect( blockText ).toBe( 'alpha beta gamma delta' ); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love these tests, thanks @notnownikki
I'm going through the linked issues and adding e2e tests where I can. I want to get an e2e test for #6021 next. So far, all is looking good! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Nice, feel free to open follow-up PRs with more e2e test 🎉 |
Description
Restores a part of #11287. It should not restore any bugginess! :)
We need to make sure none of the issues that #10799 fixed are reintroduced.
Still needs e2e test and investigate failing e2e tests.
How has this been tested?
Screenshots
Types of changes
Checklist: