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

Add URL handling to raw handler #3326

Merged
merged 1 commit into from Nov 15, 2017

Conversation

Projects
None yet
3 participants
@iseulde
Member

iseulde commented Nov 3, 2017

Description

This PR changes several things regarding the pasting of URLs.

  1. Instead of using TinyMCE's link-over-word paste, add it to Gutenberg, and add the behaviour of core which ignores whitespace and empty/wrapping HTML tags.

    https://github.com/WordPress/WordPress/blob/06fa4161aa74619239cf27017d124081c825684a/wp-includes/js/tinymce/plugins/wplink/plugin.js#L320-L336

  2. Remove the 'paste' transformation type from the patterns plugin, and instead use the raw handler. This will allow URLs to be embedded from paste and raw handling in general, not just pasting a URL on an empty line. The paste transform was kind of an odd hack in the patterns plugin.

One thing that is a bit awkward is that the order of registration matters. This is similar to the <pre> and <pre><code> tags. I'm not sure what the best solution is other than ensuring the right order (also register plugins first), or adding a condition to the paragraph block to ignore ones with a URL (bad imo).

How Has This Been Tested?

  • Paste a (e.g. YouTube) URL on an empty line. Should embed.
  • Create an old post (or similar) with a URL on its own line. Paste in Gutenberg. Should embed.
  • Paste a URL in the classic editor block. Convert to blocks. Should keep embed.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.
@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 3, 2017

Codecov Report

Merging #3326 into master will decrease coverage by 0.03%.
The diff coverage is 15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3326      +/-   ##
==========================================
- Coverage   34.65%   34.61%   -0.04%     
==========================================
  Files         260      260              
  Lines        6741     6757      +16     
  Branches     1220     1226       +6     
==========================================
+ Hits         2336     2339       +3     
- Misses       3719     3728       +9     
- Partials      686      690       +4
Impacted Files Coverage Δ
blocks/editable/patterns.js 1.85% <ø> (+0.03%) ⬆️
blocks/editable/index.js 10.23% <0%> (-0.56%) ⬇️
blocks/api/raw-handling/index.js 84.21% <100%> (+0.87%) ⬆️
blocks/library/embed/index.js 46.15% <100%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f31557...a58d720. Read the comment docs.

codecov bot commented Nov 3, 2017

Codecov Report

Merging #3326 into master will decrease coverage by 0.03%.
The diff coverage is 15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3326      +/-   ##
==========================================
- Coverage   34.65%   34.61%   -0.04%     
==========================================
  Files         260      260              
  Lines        6741     6757      +16     
  Branches     1220     1226       +6     
==========================================
+ Hits         2336     2339       +3     
- Misses       3719     3728       +9     
- Partials      686      690       +4
Impacted Files Coverage Δ
blocks/editable/patterns.js 1.85% <ø> (+0.03%) ⬆️
blocks/editable/index.js 10.23% <0%> (-0.56%) ⬇️
blocks/api/raw-handling/index.js 84.21% <100%> (+0.87%) ⬆️
blocks/library/embed/index.js 46.15% <100%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f31557...a58d720. Read the comment docs.

@iseulde iseulde requested review from mcsf and youknowriad Nov 3, 2017

@iseulde iseulde changed the title from Redo URL pasting to Add URL handling to raw handler Nov 3, 2017

@mcsf

This comment has been minimized.

Show comment
Hide comment
@mcsf

mcsf Nov 9, 2017

Contributor

One thing that is a bit awkward is that the order of registration matters. This is similar to the <pre> and <pre><code> tags. I'm not sure what the best solution is other than ensuring the right order (also register plugins first), or adding a condition to the paragraph block to ignore ones with a URL (bad imo).

This seems normal to me. I agree we shouldn't try to have extra rules in Paragraph to ignore URLs. As with any parsable system, expressions will match multiple constructs and precedence rules will be needed. Take basic algebra and precedence rules for +, -, , ÷, for instance. For now, relying on import order for the core library seems acceptable. As we start focusing on extensibility across Gutenberg, however, something more flexible will be needed—plugins will sometimes want to act before core blocks, and sometimes after them. One of the most obvious solutions would be a priority system as known in WordPress core (add_action( 'tag', 'function', 10 )).

Contributor

mcsf commented Nov 9, 2017

One thing that is a bit awkward is that the order of registration matters. This is similar to the <pre> and <pre><code> tags. I'm not sure what the best solution is other than ensuring the right order (also register plugins first), or adding a condition to the paragraph block to ignore ones with a URL (bad imo).

This seems normal to me. I agree we shouldn't try to have extra rules in Paragraph to ignore URLs. As with any parsable system, expressions will match multiple constructs and precedence rules will be needed. Take basic algebra and precedence rules for +, -, , ÷, for instance. For now, relying on import order for the core library seems acceptable. As we start focusing on extensibility across Gutenberg, however, something more flexible will be needed—plugins will sometimes want to act before core blocks, and sometimes after them. One of the most obvious solutions would be a priority system as known in WordPress core (add_action( 'tag', 'function', 10 )).

@mcsf

mcsf approved these changes Nov 9, 2017

As for the PR, LGTM! Soon we should work on documentation for the whole paste / patterns / transforms stack.

@iseulde iseulde merged commit 5255c93 into master Nov 15, 2017

3 checks passed

codecov/project 34.61% (-0.04%) compared to 3f31557
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@iseulde iseulde deleted the try/paste-links branch Nov 15, 2017

@mcsf mcsf referenced this pull request Nov 28, 2017

Merged

Pasting: Convert unknown shortcodes to Shortcode block #3610

0 of 3 tasks complete
@vedranmiletic

This comment has been minimized.

Show comment
Hide comment
@vedranmiletic

vedranmiletic Dec 16, 2017

Is this in 1.9.1? I have a blocks that has:

  • a paragraph
  • YouTube iframe embed code
  • a paragraph

And YouTube iframe embed code gets deleted upon "Convert to blocks".

vedranmiletic commented Dec 16, 2017

Is this in 1.9.1? I have a blocks that has:

  • a paragraph
  • YouTube iframe embed code
  • a paragraph

And YouTube iframe embed code gets deleted upon "Convert to blocks".

@mcsf

This comment has been minimized.

Show comment
Hide comment
@mcsf

mcsf Dec 21, 2017

Contributor

@vedranmiletic, do you mean classic (pre-Gutenberg) content? If so, I've logged issue #4125.

Contributor

mcsf commented Dec 21, 2017

@vedranmiletic, do you mean classic (pre-Gutenberg) content? If so, I've logged issue #4125.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment