Add multi-block transform functionality. #3357

jorgefilipecosta · 2017-11-06T23:04:56Z

Description

This PR's implements multi-block transform functionality. Allowing the user to transform a selection of multiple blocks into other block types. It changes blocks api. When specifying the transform the developer can set isMultiBlock to true to set that the transform is able to handle more than one block.
Multiblock transforms were already added to transform from multiple paragraphs into a list and multiple images into a gallery.

Testing

Create multiple paragraphs, select them, go to the transform area and transform them into a list.
Add multiple images select them and transform them into a gallery.

Screenshots:

codecov · 2017-11-06T23:16:54Z

Codecov Report

Merging #3357 into master will increase coverage by 0.46%.
The diff coverage is 46.34%.

@@            Coverage Diff             @@
##           master    #3357      +/-   ##
==========================================
+ Coverage    31.3%   31.76%   +0.46%     
==========================================
  Files         246      246              
  Lines        6741     6856     +115     
  Branches     1209     1243      +34     
==========================================
+ Hits         2110     2178      +68     
- Misses       3896     3932      +36     
- Partials      735      746      +11

Impacted Files	Coverage Δ
editor/block-toolbar/index.js	`0% <ø> (ø)`	⬆️
blocks/library/gallery/index.js	`31.81% <0%> (-5.03%)`	⬇️
editor/block-switcher/index.js	`0% <0%> (ø)`	⬆️
blocks/library/list/index.js	`7.95% <0%> (-0.19%)`	⬇️
editor/header/header-toolbar/index.js	`0% <0%> (ø)`	⬆️
blocks/api/factory.js	`93.54% <100%> (+2.92%)`	⬆️
editor/inserter/menu.js	`49.18% <0%> (-0.42%)`	⬇️
blocks/api/categories.js	`100% <0%> (ø)`	⬆️
editor/sidebar/post-visibility/index.js	`54.54% <0%> (+14.54%)`	⬆️
... and 1 more

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 fa59bf0...993e7fb. Read the comment docs.

youknowriad

This is really awesome, I like it.

Now that we have this, I think we should update the list -> paragraph single transformation to create multiple paragraphs, one for each list item. And for consistency, I'd also update the paragraph -> list transformation to always create a list with one item (and avoid splitting on brs) cc @jasmussen
Images to gallery is great too, but same here, we should now add a gallery => images transform.

Overall, the code looks good, I like the approach. Really great work here

youknowriad · 2017-11-07T08:17:50Z

editor/block-switcher/index.js

-		onTransform( block, name ) {
+	( state, ownProps ) => {
+		const isMultiBlock = ! ownProps.uid && getSelectedBlockCount( state ) > 1;
+		const blocks = isMultiBlock ? getMultiSelectedBlocks( state ) :


It feels a bit weird to retrive the blocks from the state when there's a multiselection and retrieve them from the uid when passed. Can we update the component to always use a uids prop To make this consistent?

youknowriad · 2017-11-07T08:18:43Z

blocks/api/test/factory.js

@@ -144,7 +144,7 @@ describe( 'block factory', () => {
 				value: 'ribs',
 			} );

-			const transformedBlocks = switchToBlockType( block, 'core/updated-text-block' );
+			const transformedBlocks = switchToBlockType( [ block ], 'core/updated-text-block' );


To avoid all these changes, we could have used lodash's castArray in the switchToBlockType function

jasmussen · 2017-11-07T08:39:36Z

Now that we have this, I think we should update the list -> paragraph single transformation to create multiple paragraphs, one for each list item. And for consistency, I'd also update the paragraph -> list transformation to always create a list with one item (and avoid splitting on brs)

This sounds good to me. Given the recent improvements in cross-block selection, and the hope that we can make multi-select transformations like selecting three paragraphs and transforming those into a list, this makes a lot of sense.

I think @mtias noted an opportunity around lists as well, that if we were to make each list item into its own block, we might be able to do some cool list sorting things as part of that.

youknowriad · 2017-11-07T09:19:57Z

This probably closes #539

youknowriad · 2017-11-07T14:19:39Z

editor/block-switcher/index.js

 	const blocksToBeTransformedFrom = reduce( getBlockTypes(), ( memo, type ) => {
 		const transformFrom = get( type, 'transforms.from', [] );
-		const transformation = find( transformFrom, t => t.type === 'block' && t.blocks.indexOf( block.name ) !== -1 );
+		const transformation = find( transformFrom, t => t.type === 'block' && t.blocks.indexOf( sourceBlockName ) !== -1 && ( ! isMultiBlock || t.isMultiBlock ) );


Minor: The line is a bit too long

youknowriad · 2017-11-07T14:22:57Z

editor/header/header-toolbar/index.js

@@ -35,6 +36,7 @@ function HeaderToolbar( { hasUndo, hasRedo, hasFixedToolbar, undo, redo } ) {
 				label={ __( 'Redo' ) }
 				disabled={ ! hasRedo }
 				onClick={ redo } />
+			{ isMultiBlockSelection && <BlockSwitcher key="switcher" uids={ selectedBlockUids } /> }


From a design perspective, this is always showing up in the Header even if I choose to show the block toolbar next to the block, I think we might want to show the switcher in the first selected block's toolbar when this option is toggled. (You can toggle this option in the ellipsis menu at the top right of the editor) cc @jasmussen

Regarding this point, I agree, adding the transform drop-down to the tools of the first block may be an option if the config preference is set. But I think adding this logic would increase the scope of this PR and make it harder to review. If it is ok for you I would prefer to add this logic in a future PR.

youknowriad

Awesome PR

jasmussen · 2017-11-07T15:55:00Z

I think the work done here is very nice, and yes it does indeed close #539.

The current mockups put the transformation into the ellipsis menu, like Riad suggests. But an argument can be made that it's okay to show this in the toolbar. If it's significantly easier to do so, we may do that as a first step before we put the transformation inside the ellipsis menu, then we can test this as an interface first, and move on to the ellipsis if we find that is more contextual and sufficiently discoverable.

Let's discuss the perspectives first, though, and @karmatosed and @mtias feel free to chime in:

We'd want to show list making chiefly on paragraph blocks. That works fine ✅
We'd want you to be able to also make blockquotes when selecting virtually any block. I say "virtually" because there might be added complexity for images that are wide or fullwide, and until we have proper nesting we'd probably want to limit this to just a few block types.
In the future it would be nice to be able to select 5 images in sequence, and transform those into a gallery.
Code or Preformatted would also be nice options to allow on blocks outside of just paragraphs.
Perhaps it would be nice to create a plugin API, so plugins could show their own buttons next to some multi selections?

None of the above have to be addressed in this PR, but it's context for deciding whether we want to move the transformations into an ellipsis now, or later. Like I said, if it's a ton of extra work, it's probably okay to try and go with this now, but at a minimum we'd want to fix this extra space that appears:

☝️ Note how the switcher moves to the left on multi select. It should align with a normal text block toolbar.

jasmussen · 2017-11-07T15:56:27Z

A quick next enhancement could be to explode the contents of the switcher dropdown into direct buttons for heading, quote, list, pre and verse, as opposed to keeping them in a dropdown.

jasmussen · 2017-11-07T15:57:22Z

Also agree: awesome PR! Thank you!

youknowriad · 2017-11-07T16:01:15Z

In the future it would be nice to be able to select 5 images in sequence, and transform those into a gallery.

This already works :)

youknowriad · 2017-11-07T16:03:57Z

whether we want to move the transformations into an ellipsis now

Actually, I was not talking about moving those to the ellipsis for now. I was just saying we should be consistent with the position of the block switcher of one selected block: It shows in the header's toolbar if the block toolbar is "fixed" and shows in the contextual toolbar if it's not.

--

That said, I think moving all these switchers to the block's ellipsis menu should be better but I also think this is a separate task :)

jorgefilipecosta · 2017-11-07T16:31:28Z

Hi @jasmussen, @youknowriad, I added a new very simple commit, that makes the space between transforms of multiple blocks and single blocks the same, solving the problem identified by @jasmussen.
There is still some work to do and improvements that should be made, but this makes multi transforms available for testing. Then I will gladly continue to iterate and address the points referred.

jasmussen · 2017-11-07T16:33:52Z

If Riad approves so do I!

jasmussen · 2017-11-07T16:36:22Z

In the future it would be nice to be able to select 5 images in sequence, and transform those into a gallery.

This already works :)

💥💥💥

jorgefilipecosta · 2017-11-07T17:52:36Z

Thank you for your inputs it was merged :) Now the work will continue to address the other points.

mtias · 2017-11-14T13:57:32Z

Great work here. I agree with moving this to the ellipsis menu for the selected group of blocks. I actually missed the transformation in the header.

jorgefilipecosta added 3 commits November 6, 2017 22:38

Added multi-block transform functionality.

e5868e8

Implemented multi block transform from images to gallery.

1b1233c

Implemented multi-block transform from paragraphs to list.

124b6d2

jorgefilipecosta added [Feature] Block API API that allows to express the block paradigm. [Feature] Blocks Overall functionality of blocks [Type] Enhancement A suggestion for improvement. labels Nov 6, 2017

jorgefilipecosta self-assigned this Nov 6, 2017

youknowriad reviewed Nov 7, 2017

View reviewed changes

jorgefilipecosta force-pushed the add/multi-block-transforms branch 2 times, most recently from d3c44a4 to 06a86ae Compare November 7, 2017 12:55

jorgefilipecosta added 3 commits November 7, 2017 14:16

switchToBlockType api changes.

c11319c

Improved list to paragraph and paragraph to list transformations.

9c11036

Implemented gallery to images transform.

3353717

jorgefilipecosta force-pushed the add/multi-block-transforms branch from 3abd85e to 3353717 Compare November 7, 2017 14:16

youknowriad reviewed Nov 7, 2017

View reviewed changes

Line size fixes

3ce3898

youknowriad approved these changes Nov 7, 2017

View reviewed changes

Solved white space difference on multi-block transform area.

993e7fb

jorgefilipecosta force-pushed the add/multi-block-transforms branch from 08985c8 to 993e7fb Compare November 7, 2017 16:16

jorgefilipecosta merged commit d37ad46 into master Nov 7, 2017

jorgefilipecosta deleted the add/multi-block-transforms branch November 7, 2017 17:51

ellatrix mentioned this pull request Jan 17, 2018

Add block-level keyboard shortcuts #4436

Closed

3 tasks

mcsf mentioned this pull request Jan 27, 2018

Try: Improve round-trip transformation of text and quote blocks #1774

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi-block transform functionality. #3357

Add multi-block transform functionality. #3357

jorgefilipecosta commented Nov 6, 2017 •

edited

Loading

codecov bot commented Nov 6, 2017 •

edited

Loading

youknowriad left a comment

youknowriad Nov 7, 2017

youknowriad Nov 7, 2017

jasmussen commented Nov 7, 2017

youknowriad commented Nov 7, 2017

youknowriad Nov 7, 2017

youknowriad Nov 7, 2017

jorgefilipecosta Nov 7, 2017

youknowriad left a comment

jasmussen commented Nov 7, 2017 •

edited

Loading

jasmussen commented Nov 7, 2017

jasmussen commented Nov 7, 2017

youknowriad commented Nov 7, 2017

youknowriad commented Nov 7, 2017

jorgefilipecosta commented Nov 7, 2017

jasmussen commented Nov 7, 2017

jasmussen commented Nov 7, 2017 •

edited

Loading

jorgefilipecosta commented Nov 7, 2017

mtias commented Nov 14, 2017

Add multi-block transform functionality. #3357

Add multi-block transform functionality. #3357

Conversation

jorgefilipecosta commented Nov 6, 2017 • edited Loading

Description

Testing

Screenshots:

codecov bot commented Nov 6, 2017 • edited Loading

Codecov Report

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad Nov 7, 2017

Choose a reason for hiding this comment

youknowriad Nov 7, 2017

Choose a reason for hiding this comment

jasmussen commented Nov 7, 2017

youknowriad commented Nov 7, 2017

youknowriad Nov 7, 2017

Choose a reason for hiding this comment

youknowriad Nov 7, 2017

Choose a reason for hiding this comment

jorgefilipecosta Nov 7, 2017

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

jasmussen commented Nov 7, 2017 • edited Loading

jasmussen commented Nov 7, 2017

jasmussen commented Nov 7, 2017

youknowriad commented Nov 7, 2017

youknowriad commented Nov 7, 2017

jorgefilipecosta commented Nov 7, 2017

jasmussen commented Nov 7, 2017

jasmussen commented Nov 7, 2017 • edited Loading

jorgefilipecosta commented Nov 7, 2017

mtias commented Nov 14, 2017

jorgefilipecosta commented Nov 6, 2017 •

edited

Loading

codecov bot commented Nov 6, 2017 •

edited

Loading

jasmussen commented Nov 7, 2017 •

edited

Loading

jasmussen commented Nov 7, 2017 •

edited

Loading