Skip to content

Prevent cursor jumps inside contenteditable#2701

Merged
marvinhagemeister merged 7 commits into
masterfrom
bugfix/2691
Aug 30, 2020
Merged

Prevent cursor jumps inside contenteditable#2701
marvinhagemeister merged 7 commits into
masterfrom
bugfix/2691

Conversation

@sventschui

Copy link
Copy Markdown
Member

This prevents cursor jumps inside contenteditable. It seems a bit expensive to check for innerHTML equality but the only way I see to get this fixed.

Fixes #2691

@github-actions

github-actions Bot commented Aug 17, 2020

Copy link
Copy Markdown

Size Change: +32 B (0%)

Total Size: 40.4 kB

Filename Size Change
dist/preact.js 3.91 kB +5 B (0%)
dist/preact.min.js 3.94 kB +9 B (0%)
dist/preact.module.js 3.94 kB +10 B (0%)
dist/preact.umd.js 3.98 kB +8 B (0%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.26 kB 0 B
compat/dist/compat.module.js 3.29 kB 0 B
compat/dist/compat.umd.js 3.33 kB 0 B
debug/dist/debug.js 3.09 kB 0 B
debug/dist/debug.module.js 3.09 kB 0 B
debug/dist/debug.umd.js 3.18 kB 0 B
devtools/dist/devtools.js 185 B 0 B
devtools/dist/devtools.module.js 195 B 0 B
devtools/dist/devtools.umd.js 261 B 0 B
hooks/dist/hooks.js 1.1 kB 0 B
hooks/dist/hooks.module.js 1.12 kB 0 B
hooks/dist/hooks.umd.js 1.18 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action


// dispatch a change event
editable.innerHTML = 'World';
editable.dispatchEvent(createEvent('input'));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm I don't think I understand this test. How can a real user fire an input event or change the innerHTML but simultaneously keep text selection. My understanding is that all browsers would clear the text selection the moment the user presses a key while selecting text.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah you're right. I forgot to get back to this. I originally wanted to use the range to place the caret (see https://stackoverflow.com/a/6249440/2164347) but failed. I will try again to make the test clear. In the end this just makes sure we do not touch innerHTML.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I modified the test to be more realistic. It now ensures the caret stays where it was prior to the input event

@github-actions

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 99.803% when pulling 9163efb on bugfix/2691 into 6ab49d9 on master.

@wbercx

wbercx commented Aug 29, 2020

Copy link
Copy Markdown

Awesome work! This seems to take draft-js from being completely unusable to actually working.

@marvinhagemeister marvinhagemeister left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is amazing 🙌

@marvinhagemeister marvinhagemeister merged commit 2c6e478 into master Aug 30, 2020
@marvinhagemeister marvinhagemeister deleted the bugfix/2691 branch August 30, 2020 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

contenteditable broken with Preact 10

5 participants