Add "Convert to blocks" option in HTML block #7667

oandregal · 2018-07-02T15:15:17Z

Addresses #7466

I've tested this by trying some HTML with a direct map to blocks (ie: lists, paragraphs), and also HTML that has no direct counterpart as a WordPress block. As far as this relies on rawHandler for block conversion, they're in line with what Gutenberg can manage.

aduth

I could see a pattern emerging here where a block wants to add options to its own "More" menu, so instead of adding awareness of this to the editor in the BlockSettingsMenu component, I wonder if this should be a Slot/Fill pairing similar to what we're doing with InspectorControls.

aduth · 2018-07-02T15:50:12Z

Related: #7199

oandregal · 2018-07-03T12:02:46Z

Thanks for pointing me to that debate, Andrew. After reading the thread, my current thinking is: let's merge this and iterate on the more general menu extensibility in a separate PR.

The concerns are different, and from the convo with @youknowriad @gziolo I got the feeling that the menu extensibility still needs some discussion. I will prepare a separate PR for that and will also look at the code of the general menu that already includes some space for plugins.

gziolo · 2018-07-03T12:07:19Z

editor/components/block-settings-menu/html-converter.js

+	withDispatch( ( dispatch ) => ( {
+		onReplace: dispatch( 'core/editor' ).replaceBlocks,
+	} ) ),
+	withAPIData( ( { postType } ) => ( {


We are trying to get rid of withAPIData usage and deprecate it as explained in #7390 and #7397. An alternative approach would be to use _links object returned from the REST API as implemented in other places by @danielbachhuber, examples: #7495 or even better #6724.

It can be done in a separate PR given that there are a few occurrences of get( user, [ 'data', 'capabilities', 'unfiltered_html' ], false ), but this should be started in parallel.

Also related #6361

Fixed in 138c7f1.

Also added the canUserUseUnfilteredHTML selector in d0e747e to substitute the rest of occurrences in a separate PR once this lands.

#7729 substitutes the remaining cases of unfiltered_html and withAPIData. It depends on this being merged.

gziolo · 2018-07-03T12:09:14Z

This newly introduced component is very similar to UnknownConverter (https://github.com/WordPress/gutenberg/blob/6de5a3bc7f0e940b5e00a26609b7e286bfd60fb7/editor/components/block-settings-menu/unknown-converter.js) - would be there a way to consolidate them?

oandregal · 2018-07-03T12:35:55Z

re: UnknownConverter and HTMLConverter. I've considered other approaches:

using a single Converter that would use conditionals to differentiate different block logic.
extract a common Converter that HTMLConverter and UnknownConverter would inherit from using the Template pattern.
create a mechanism for blocks to register their own converter, allowing extensibility.

After doing an audit of blocks #7684 I operate with this assumption: Convert to blocks is a transitional feature that it's supposed to be used by the classic and HTML blocks and not plugins. Given that assumption, we don't expect to create more converters (for core or plugins). So, at this point, building any indirection to allow extensibility could do more harm than good (perhaps less code, but also less obvious).

See #6361 for reference

gziolo

It works great. I left some minor comments to address before merging this PR.

Thanks for pointing me to that debate, Andrew. After reading the thread, my current thinking is: let's merge this and iterate on the more general menu extensibility in a separate PR.

Should we open a follow-up issue where we could discuss how to make this part of the extensibility layer, to allow plugin developers add their own option in a similar fashion?

gziolo · 2018-07-10T08:43:20Z

editor/components/block-settings-menu/html-converter.js

+
+	const label = __( 'Convert to blocks' );
+
+	const convertToBlocks = () => {


Is it possible to move it to withDispatch or is there any blocker?

I guess it could be refactored, indeed. I don't mind one-way or the other and did that to mimic what existed in UnknownConverter. I'll leave it as it is, we can always refactor both later if necessary.

It's a leaf component inside the dropdown which is not rendered by default, so this shouldn't impact performance. In general, having a local function inside render method isn't expected because it causes unnecessary re-renders.

Grzegorz, #7885 follows up the work on this PR and addresses some ideas you mentioned in this thread (share common code, move to withDispatch).

gziolo · 2018-07-10T08:44:37Z

editor/store/selectors.js

@@ -1891,3 +1891,14 @@ export function getTokenSettings( state, name ) {

 	return state.tokens[ name ];
 }
+
+/*
+ * Returns whether or not the user has the unfiltered_html capability


Nit: Comment should start with /**. All sentences should end with . :)

Let's merge this one and iterate.

oandregal · 2018-07-10T12:36:00Z

Thanks for the review, Grzegorz!

Should we open a follow-up issue where we could discuss how to make this part of the extensibility layer, to allow plugin developers add their own option in a similar fashion?

I'm already working on this and will ping you soon with a proposal.

gziolo · 2018-07-10T12:41:01Z

@nosolosw, congrats. You made your first Gutenberg contribution! 🎉

oandregal self-assigned this Jul 2, 2018

oandregal force-pushed the add/html-to-blocks branch from 3aca796 to 6de5a3b Compare July 2, 2018 15:16

aduth previously requested changes Jul 2, 2018

View reviewed changes

aduth added [Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Paste labels Jul 2, 2018

gziolo reviewed Jul 3, 2018

View reviewed changes

oandregal added 4 commits July 5, 2018 12:29

Allow converting core/html block to blocks

b59f27d

Add unfiltered_html capability to targetSchema

6fe9878

Take capability from _links property in post object

138c7f1

See #6361 for reference

Add canUserUseUnfilteredHTML selector

d0e747e

oandregal force-pushed the add/html-to-blocks branch from 6de5a3b to d0e747e Compare July 5, 2018 14:56

oandregal added 4 commits July 5, 2018 17:08

Remove unused import

f9ad80b

Add selector tests

edbf3a3

Add REST API tests

7a44a2e

Add REST API tests for multisite

8d79467

oandregal mentioned this pull request Jul 5, 2018

Take the unfiltered_html capability from state #7729

Merged

Make php linter happy

ba4532c

gziolo approved these changes Jul 10, 2018

View reviewed changes

Fix jsdoc comment

570aa39

oandregal merged commit ccbb54b into master Jul 10, 2018

oandregal mentioned this pull request Jul 10, 2018

Update/use unfiltered html selector #7865

Merged

gziolo added this to the 3.3 milestone Jul 10, 2018

This was referenced Jul 10, 2018

Avoid direct use of user capabilities in client-side code #6361

Closed

Update/refactor converters #7885

Merged

ZebulanStanphill mentioned this pull request Jul 13, 2018

Allow convert to blocks for other block types #4795

Closed

danielbachhuber mentioned this pull request Sep 30, 2018

Use dashes instead of underscores for consistency with other actions #10256

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Convert to blocks" option in HTML block #7667

Add "Convert to blocks" option in HTML block #7667

oandregal commented Jul 2, 2018 •

edited

Loading

aduth left a comment

aduth commented Jul 2, 2018

oandregal commented Jul 3, 2018

gziolo Jul 3, 2018

gziolo Jul 3, 2018

danielbachhuber Jul 3, 2018

oandregal Jul 5, 2018

oandregal Jul 5, 2018 •

edited

Loading

gziolo commented Jul 3, 2018 •

edited

Loading

oandregal commented Jul 3, 2018

gziolo left a comment

gziolo Jul 10, 2018

oandregal Jul 10, 2018

gziolo Jul 10, 2018

oandregal Jul 11, 2018

gziolo Jul 10, 2018

oandregal Jul 10, 2018

oandregal commented Jul 10, 2018

gziolo commented Jul 10, 2018


		const label = __( 'Convert to blocks' );

		const convertToBlocks = () => {

Add "Convert to blocks" option in HTML block #7667

Add "Convert to blocks" option in HTML block #7667

Conversation

oandregal commented Jul 2, 2018 • edited Loading

aduth left a comment

Choose a reason for hiding this comment

aduth commented Jul 2, 2018

oandregal commented Jul 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal Jul 5, 2018 • edited Loading

Choose a reason for hiding this comment

gziolo commented Jul 3, 2018 • edited Loading

oandregal commented Jul 3, 2018

gziolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Jul 10, 2018

gziolo commented Jul 10, 2018

oandregal commented Jul 2, 2018 •

edited

Loading

oandregal Jul 5, 2018 •

edited

Loading

gziolo commented Jul 3, 2018 •

edited

Loading