Keep the cursor position in the Classic Block by azaozz · Pull Request #12393 · WordPress/gutenberg

azaozz · 2018-11-28T19:03:08Z

On blur store the cursor position and the "raw" content of the Classic Block.
Use the "raw" content when refreshing the editor content on componentDidUpdate(). It contains the cursor position "bookmark".
Restore the cursor position on focus and before inserting content.

- On blur store the cursor position and the "raw" content of the Classic Block. - Use the "raw" content when refreshing the editor content on `componentDidUpdate()`. - Restore the cursor position on focus and before inserting content.

youknowriad · 2018-11-28T19:14:48Z

packages/block-library/src/classic/edit.js

-
+		const { clientId, attributes } = this.props;
 		const editor = window.tinymce.get( `editor-${ clientId }` );
+		let { contentRaw } = attributes;


contentRaw is not persisted, we should probably use "local state" for it.

Ah, sure, that'd work even better :)

youknowriad

We could use some e2e test for the classic block :)

Needed when the first thing the user does after adding a Classic Block is to open a modal that inserts something, like the media modal.

ellatrix · 2018-11-29T11:05:45Z

packages/block-library/src/classic/edit.js

+
+			// Keep the "raw" content as it contains the caret position bookmark.
 			setAttributes( {
+				contentRaw: editor.getContent( { format: 'raw' } ),


Storing the raw content without any processing seems like a bad idea? Why should this be different from the classic editor?

ellatrix · 2018-11-29T11:06:50Z

packages/block-library/src/classic/edit.js

+		editor.on( 'init', () => {
+			// Store the initial "raw" content.
+			setAttributes( {
+				contentRaw: editor.getContent( { format: 'raw' } ),


I wouldn't expect merely initialising the block to change the content. The user didn't do anything.

ellatrix · 2018-11-29T11:07:15Z

packages/block-library/src/classic/edit.js

 		const editor = window.tinymce.get( `editor-${ clientId }` );
+		let { contentRaw } = attributes;
+
+		// Strip white space before comparing to avoid resetting the editor content when not necessary;


What kind of whitespace is different?

The only differences I'm seeing are in some <p>\u00a0</p> and some ; missing/being added to ends of inline style attributes.

azaozz · 2018-11-29T11:12:30Z

Looking more at this, still can't figure out what the refresh code in componentDidUpdate() is supposed to do:

const { clientId, attributes: { content } } = this.props;
....
if ( prevProps.attributes.content !== content ) {
	editor.setContent( content || '' );
}

Seems at some point it was possible that attributes.contend was changed/updated outside the editor, but that doesn't seem to be the case now. Perhaps this is a "left over" from how undo/redo was working previously?

Even if this is still needed, don't think that looking at prevProps.attributes.content is a good way to determine what's in the editor at that moment. It needs to do editor.getContent().

@iseulde @aduth any chance you could remember what was the purpose of the above code? I know, it's been there for over a year...

ellatrix · 2018-11-29T11:16:04Z

To me, this is an interesting thing that happens in master:

Load a classic block with some text. Place the caret somewhere other than the start.
Click the media button in the toolbar.
Select an image.
Close the media modal. Do not insert it!
Place the caret back where it was (somewhere other than the very start).
Click the media button in the toolbar.
The image you selected last time will already be selected. Do not click it again!
Insert the already selected image.
The image is correctly inserted.

This means that clicking the image specifically is causing the focus loss?

ellatrix · 2018-11-29T11:17:55Z

Seems at some point it was possible that attributes.contend was changed/updated outside the editor, but that doesn't seem to be the case now.

I think it is still needed for undo/redo. It needs to be possible to set the content from outside the classic block.

Even if this is still needed, don't think that looking at prevProps.attributes.content is a good way to determine what's in the editor at that moment. It needs to do editor.getContent().

Sounds good to me.

azaozz · 2018-11-29T11:19:59Z

This means that clicking the image specifically is causing the focus loss?

Yes, clicking anywhere outside the editor triggers the blur() and looses the caret position in the editor body <div>.

That is easily fixed by doing the getBookmark(), moveToBookmark(). However these get lost/reset when componentDidUpdate() sometimes.

azaozz · 2018-11-29T11:23:38Z

I think it is still needed for undo/redo.

I'm probably missing something but can't trigger that... Disabling the revert of the content doesn't seem to break undo/redo.

ellatrix · 2018-11-29T11:29:54Z

Another interesting thing: even if I remove componentDidUpdate entirely, the problem still exists. Again only happens if you select the image in the media library. Otherwise not.

azaozz · 2018-11-29T11:31:06Z

even if I remove componentDidUpdate entirely, the problem still exists

Right. It still needs getBookmark(), moveToBookmark() on blur/focus.

ellatrix · 2018-11-29T11:31:57Z

Right. It still needs getBookmark(), moveToBookmark() on blur/focus.

Why does this only happen for selecting an image though? What's special about this image?

azaozz · 2018-11-29T11:33:42Z

Here it happens for any click that focuses something outside the editor body. For example opening the menu, etc.

azaozz · 2018-11-29T12:17:44Z

Found it :)

The check in componentDidUpdate() is needed only when the global undo/redo buttons are clicked. Not needed/not running for Ctrl/Cmd + z though...

azaozz · 2018-11-29T14:06:41Z

Closing this in favor of #12415.

Keep the cursor position on blur

5344f2b

- On blur store the cursor position and the "raw" content of the Classic Block. - Use the "raw" content when refreshing the editor content on `componentDidUpdate()`. - Restore the cursor position on focus and before inserting content.

azaozz requested review from ellatrix and youknowriad November 28, 2018 19:03

azaozz added the [Block] Classic Affects the Classic Editor Block label Nov 28, 2018

azaozz added this to the 4.6 milestone Nov 28, 2018

youknowriad reviewed Nov 28, 2018

View reviewed changes

azaozz mentioned this pull request Nov 28, 2018

Classic Block - Insert Media Icon - New Upload jumps to top #10509

Closed

Store the initial "raw" content on editor init

36073f5

Needed when the first thing the user does after adding a Classic Block is to open a modal that inserts something, like the media modal.

ellatrix reviewed Nov 29, 2018

View reviewed changes

ellatrix mentioned this pull request Nov 29, 2018

Classic Block: set correct focus back after blur #12415

Merged

4 tasks

youknowriad modified the milestones: 4.6, 4.7 Nov 29, 2018

azaozz closed this Nov 29, 2018

azaozz deleted the fix/classic-block-cursor-position-on-blur branch November 29, 2018 14:07

MaxMood96 mentioned this pull request Mar 7, 2024

[Snyk] Security upgrade webdriverio from 8.16.20 to 8.33.0 MaxMood96/gutenberg#146

Open

naiba4 mentioned this pull request Mar 8, 2024

[Snyk] Security upgrade webdriverio from 8.16.20 to 8.33.0 naiba4/gutenberg#47

Open

naiba4 mentioned this pull request Apr 22, 2024

[Snyk] Fix for 14 vulnerabilities naiba4/gutenberg#53

Open

Conversation

azaozz commented Nov 28, 2018

Uh oh!

youknowriad Nov 28, 2018

Choose a reason for hiding this comment

Uh oh!

azaozz Nov 28, 2018

Choose a reason for hiding this comment

Uh oh!

youknowriad left a comment

Choose a reason for hiding this comment

Uh oh!

ellatrix Nov 29, 2018

Choose a reason for hiding this comment

Uh oh!

ellatrix Nov 29, 2018

Choose a reason for hiding this comment

Uh oh!

ellatrix Nov 29, 2018

Choose a reason for hiding this comment

Uh oh!

azaozz Nov 29, 2018

Choose a reason for hiding this comment

Uh oh!

azaozz commented Nov 29, 2018

Uh oh!

ellatrix commented Nov 29, 2018

Uh oh!

ellatrix commented Nov 29, 2018

Uh oh!

azaozz commented Nov 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azaozz commented Nov 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellatrix commented Nov 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azaozz commented Nov 29, 2018

Uh oh!

ellatrix commented Nov 29, 2018

Uh oh!

azaozz commented Nov 29, 2018

Uh oh!

azaozz commented Nov 29, 2018

Uh oh!

azaozz commented Nov 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

azaozz commented Nov 29, 2018 •

edited

Loading

azaozz commented Nov 29, 2018 •

edited

Loading

ellatrix commented Nov 29, 2018 •

edited

Loading