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

[Tiny PR] Paste: do not force blocks for empty rich text #17140

Merged
merged 2 commits into from Sep 4, 2019

Conversation

@ellatrix
Copy link
Member

commented Aug 22, 2019

Description

Fixes #16990. There's probably similar older issues.
Also blocks introducing caption splitting as you wouldn't be able to paste in an empty caption.

Reverts some of #3326, introduced 2 years ago for embedding on URL paste. I've solved that problem differently here.

How has this been tested?

See #16990. Create an empty heading. Copy paste a piece of the URL in the address bar. Expect a heading with the pasted text. On master it would be converted to a paragraph.
Paste an embeddable URL in an empty paragraph. It should be converted to an embed block.

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 marked this pull request as ready for review Aug 22, 2019
@ellatrix ellatrix force-pushed the try/empty-paste-auto branch from 3d98c3e to 80173db Aug 22, 2019
@ellatrix ellatrix requested review from ajitbohra, Soean and talldan as code owners Aug 22, 2019
@ellatrix ellatrix changed the title Paste: do not force blocks for empty rich text [Tiny PR] Paste: do not force blocks for empty rich text Aug 22, 2019
@oxyc

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2019

I can confirm (with limited testing) that this fixes my issue.

Copy link
Contributor

left a comment

Testing this branch using the same techniques I used in #16990 (comment) works as expected.

I don't know enough about __unstableEmbedURLOnPaste or the rich text component to provide a code review I'm afraid. If I find myself at a loose end, I'll dive in to the code to see if I can figure it out but I can't promise that will happen.

@@ -195,6 +195,7 @@ class ParagraphBlock extends Component {
onRemove={ onReplace ? () => onReplace( [] ) : undefined }
aria-label={ content ? __( 'Paragraph block' ) : __( 'Empty block; start writing or type forward slash to choose a block' ) }
placeholder={ placeholder || __( 'Start writing or type / to choose a block' ) }
__unstableEmbedURLOnPaste

This comment has been minimized.

Copy link
@desaiuditd

desaiuditd Aug 26, 2019

Member

Is this prop newly introduced in this PR? (Could not find its reference anywhere else in the codebase.) What is its purpose?

Not sure, what's the convention for such unstable new props. But can we add some doc on this? Maybe inline comment, readme, somewhere else? Maybe better prop naming to reflect its use case better?

This comment has been minimized.

Copy link
@ellatrix

ellatrix Sep 4, 2019

Author Member

I can provide an inline comment, but I thought the name is quite clear: it enables embedding when the user pastes a URL. This prop should normally only be used on the core paragraph block. We shouldn't document unstable props in a readme file.

An unstable API is one which serves as a means to an end. It is not desired to ever be converted into a public API.

https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/

Copy link
Contributor

left a comment

I've done some further research on paste handling and modes since my earlier comment. Now feel confident enough to approve this.

Further testing based on my enhanced knowledge worked.

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

@peterwilsoncc Thank you very much! I'm happy you felt like exploring raw handling. :)

@ellatrix ellatrix force-pushed the try/empty-paste-auto branch from 80173db to eca8fc0 Sep 4, 2019
@ellatrix ellatrix merged commit 5774046 into master Sep 4, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@ellatrix ellatrix deleted the try/empty-paste-auto branch Sep 4, 2019
@youknowriad youknowriad added this to the Gutenberg 6.5 milestone Sep 14, 2019
dd32 pushed a commit to dd32/gutenberg that referenced this pull request Sep 27, 2019
…7140)

* Paste: do not force blocks for empty rich text

* Enable embed on URL paste in paragraph block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.