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 paste after focus #16857

Merged
merged 2 commits into from
Aug 2, 2019
Merged

Fix paste after focus #16857

merged 2 commits into from
Aug 2, 2019

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Aug 1, 2019

Description

Fixes #15724.

This is a small fix for the bug that duplicates the rich text contents after pasting in a rich text instance that just received focus.

How has this been tested?

Copy a small amount of text.
Open the Gutenberg demo content.
Place the caret at the end of the first paragraph.
Move selection out of the window: e.g. place the caret in the address bar.
Place the caret again at the end of the first paragraph.
Now paste the text.
No content should be duplicated.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix added the [Package] Rich text /packages/rich-text label Aug 1, 2019
@gziolo gziolo added the [Type] Bug An existing feature does not function as intended label Aug 1, 2019
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I tested the exact same action as posted in the issue and I could reproduce it on master but not in this branch. Great change 💯

I tested pasting at the beginning, in the middle and at the end of the Paragraph block. Tried also with Heading and List blocks.

@ellatrix
Copy link
Member Author

ellatrix commented Aug 2, 2019

I’ll attempt to create an e2e test in just a bit. Thanks for the review!

@gziolo gziolo merged commit 2beadcd into master Aug 2, 2019
@gziolo gziolo deleted the fix/paste-after-focus branch August 2, 2019 07:11
@gziolo
Copy link
Member

gziolo commented Aug 2, 2019

I’ll attempt to create an e2e test in just a bit. Thanks for the review!

Sounds great, I merged this change as I want to play again with Puppeteer upgrade and this might (or not) help with some of the intermittent issues :)

@gziolo gziolo added this to the Gutenberg 6.3 milestone Aug 2, 2019
@ellatrix
Copy link
Member Author

ellatrix commented Aug 2, 2019

It doesn't seem possible to me to create an e2e test for this fix.

	it.only( 'should update internal selection after fresh focus', async () => {
		await page.keyboard.press( 'Enter' );
		await page.keyboard.type( '1' );
		// Focus the address bar (move focus out of the window).
		await pressKeyWithModifier( 'primary', 'l' );
		// Focus the rich text instance again (not at the start).
		await page.click( '.wp-block-paragraph', { button: 'middle' } );
		// Should internally set bold at selection.
		await pressKeyWithModifier( 'primary', 'b' );
		// "2" should be bold.
		await page.keyboard.type( '2' );

		expect( await getEditedPostContent() ).toMatchSnapshot();
	} );

Doing this this manually fails, but in Puppeteer it succeeds. :)
Perhaps in some cases the selectionchange event is triggered, I'm not sure.

@ellatrix
Copy link
Member Author

ellatrix commented Aug 2, 2019

Blurring the element seems to trigger it to fail. And it succeeds with this branch. Yay!

@ellatrix
Copy link
Member Author

ellatrix commented Aug 2, 2019

@gziolo I doubt it will fix the intermittent failures. Puppeteer won't wait for an animation frame. Let me think about that. :D

ellatrix added a commit that referenced this pull request Aug 6, 2019
gziolo pushed a commit that referenced this pull request Aug 7, 2019
* Update demo test to make it work with Vimeo mock

* Add e2e test

* Update selection on mouseup/keyup/touchend

* Try to fix e2e tests

* Attempt to upgrade Puppeteer

* Fix package-lock.json file

* Fix preview e2e

* Update snapshot

* Stabilise block hierarchy tests

* Remove new e2e test

* Simplify loop

* Remove puppeteer from devDependencies

* Remove cancelAnimationFrame

* Add e2e test for #16857

* Restore animation frame fix
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Fix paste after focus

* Fixed type in comment
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Update demo test to make it work with Vimeo mock

* Add e2e test

* Update selection on mouseup/keyup/touchend

* Try to fix e2e tests

* Attempt to upgrade Puppeteer

* Fix package-lock.json file

* Fix preview e2e

* Update snapshot

* Stabilise block hierarchy tests

* Remove new e2e test

* Simplify loop

* Remove puppeteer from devDependencies

* Remove cancelAnimationFrame

* Add e2e test for #16857

* Restore animation frame fix
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Fix paste after focus

* Fixed type in comment
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Update demo test to make it work with Vimeo mock

* Add e2e test

* Update selection on mouseup/keyup/touchend

* Try to fix e2e tests

* Attempt to upgrade Puppeteer

* Fix package-lock.json file

* Fix preview e2e

* Update snapshot

* Stabilise block hierarchy tests

* Remove new e2e test

* Simplify loop

* Remove puppeteer from devDependencies

* Remove cancelAnimationFrame

* Add e2e test for #16857

* Restore animation frame fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Rich text /packages/rich-text [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RichText: copy paste inside same field sometimes duplicates text
3 participants