Allow pasting tables with list content inside the cells. #46775

mpkelly · 2022-12-23T19:13:17Z

(recreated this PR from #46512 after polluting the commit log)

What?

It's based on requirement 2 from #45774. If you paste a table which includes ul or ol elements, these elements will be converted to simple content and formatted with whitespace to try and keep the original list structure. This avoids having to support complex nested content in tables.

Why?

Increase parity between Gutenberg and other editors.

How?

Allow the list schema to be embedded into td and th. Add a transform function for tables that can convert these tables into simple content that is already supported in table cells.

Testing Instructions

Check out this branch and create a table in Google Docs that has ordered or unordered in its cells. See the screenshots below.
Paste the table into Gutenberg to see the result
Try adding content before/after the list.
Try nesting lists to any depth.

Testing Instructions for Keyboard

Screenshots or screencast

Source table (Google docs)

Gutenberg editor after pasting the table above

How the td which contained the list looks in the DOM when previewing

Example of nested order lists

Google Docs

Gutenberg

… is converted to simple content which is formatted to look like a list using whitespace.

mpkelly · 2022-12-23T19:15:00Z

Copying @danielbachhuber's comment here from the old PR.

Sweet! This seems to work pretty well to me from a product perspective. I'll defer to others on the codes.

For Example 4 (below), do we want to alternate between letters and numbers?

Example 1

Google Docs

Paste Into Gutenberg

Example 2

Google Docs

Gutenberg Paste

Example 3

Google Docs

Paste Into Gutenberg

Example 4

Google Docs

Paste Into Gutenberg

Example 5

Google Docs

Paste Into Gutenberg

Originally posted by @danielbachhuber in #46512 (review)

mpkelly · 2022-12-23T19:15:26Z

For Example 4 (below), do we want to alternate between letters and numbers?

@danielbachhuber, I added some logic to do this.

danielbachhuber · 2023-01-03T14:12:54Z

FYI - I merged trunk because I had troubles with npm install

danielbachhuber

This is looking really cool, @mpkelly. Thanks for your continued work on it!

One last issue I noticed: when indentation decreases again, the switch between alpha and numeric is incorrect.

Google Doc

Paste into Gutenberg

Edit: it also seems like there's a space character inserted unexpectedly:

Maybe it would be helpful to further abstract the code so we could have unique unit tests against all of the various list HTML -> text transformations?

danielbachhuber · 2023-01-05T12:19:15Z

Requested feedback on the overall approach: https://wordpress.slack.com/archives/C02QB2JS7/p1672921120174529

mpkelly · 2023-01-11T13:59:31Z

Thanks for the feedback request, @danielbachhuber. I also tried to get some here. I get the feeling not everyone is cool with this change. Maybe the way I explained it in the PR is making it scarier than it sounds.

I have fixed that glaring bug so the numeric bullets now work. I will remove the space and then look at the tests.

danielbachhuber · 2023-01-11T14:06:35Z

I get the feeling not everyone is cool with this change.

@mpkelly Out of curiosity, what gives you that sense? I don't have a similar sense...

mpkelly · 2023-01-11T14:13:31Z

I get the feeling not everyone is cool with this change.

@mpkelly Out of curiosity, what gives you that sense? I don't have a similar sense...

There was an earlier PR (two actually), which proposed adding the same thing, so this change has been on the table for a good while now, but there hasn't been much interest outside of us two. Maybe I'm just not used to the pace of open-source projects.

annezazu · 2023-01-11T15:53:35Z

@ellatrix might you have time to review this? You come to mind as a go-to expert for these sorts of PRs related to the writing experience, including raw handling.

ellatrix · 2023-01-25T19:54:45Z

packages/block-library/src/table/transforms.js

 const tableContentPasteSchema = ( { phrasingContentSchema } ) => ( {
 	tr: {
 		allowEmpty: true,
 		children: {
 			th: {
 				allowEmpty: true,
-				children: phrasingContentSchema,
+				children: getListContentSchema( { phrasingContentSchema } ),


I'm not sure I like this. Why not use '*' for children and let raw handling parse the cell contents as blocks, then transform that to the markdown syntax. OR instead of parsing as blocks, just convert the HTML to mark down?

gutenberg/packages/block-library/src/quote/transforms.js

Lines 38 to 61 in 6517008

{

type: 'raw',

schema: () => ( {

blockquote: {

children: '*',

},

} ),

selector: 'blockquote',

transform: ( node, handler ) => {

return createBlock(

'core/quote',

// Don't try to parse any `cite` out of this content.

// * There may be more than one cite.

// * There may be more attribution text than just the cite.

// * If the cite is nested in the quoted text, it's wrong to

// remove it.

{},

handler( {

HTML: node.innerHTML,

mode: 'BLOCKS',

} )

);

},

},

Suggested change

children: getListContentSchema( { phrasingContentSchema } ),

children: '*',

Although, when it's parsed as block, it's harder to convert all blocks to something textual (like heading to plain text etc.)

Why don't we build this into the paste handler and generally convert lists to plain text if the allowed children is phrasingContentSchema? This behaviour could be useful in other places. What if there's a list in a caption? Generalising this would be great.

So inside the paste handler, I think we need a filter before removeInvalidHTML that converts list that are not allowed in these places (not in schema) to the text version of the list.

Let me know if you need help here.

ellatrix · 2023-01-25T20:07:03Z

packages/block-library/src/table/transforms.js

+		row.cells.forEach( ( cell ) => {
+			transformContent( cell );
+		} );
+	} );


Why not transform node before running getBlockAttributes? That way we don't need to parse the HTML of every cell.

ellatrix

Rephrasing my earlier comments:

The idea is good! We should just generalise it more inside of the paste handler (could be useful elsewhere like captions). This list format is nice in places where the list element is not part of the schema (and it should remain omitted from the schema in the table block).

jordesign · 2023-07-28T01:03:36Z

Just checking in on #45774 - and wanted to see how progress was going on this PR?

ellatrix

@mpkelly are you planning to continue work on this PR? Otherwise I can create an alternative.

What I'd like to see: change filterInlineHTML inside the paste handler in the blocks package to convert the lists to a pseudo inline list so this behaviour is generalised and works for all blocks (not just tables, but also captions etc.)

mpkelly · 2023-10-10T05:39:32Z

@mpkelly are you planning to continue work on this PR? Otherwise I can create an alternative.

@ellatrix, I won't work on it until next week at the earliest, so go ahead and implement your recommendation instead if you have bandwidth.

ellatrix · 2023-10-10T10:17:14Z

@mpkelly In that case, I'll wait for you to adjust it. :)

mpkelly · 2023-10-26T06:36:32Z

Thanks, @ellatrix. I am going to work on this today. I will follow your advice regarding filterInlineHTML.

mpkelly added 2 commits December 23, 2022 17:19

Allow pasting tables with list content inside the cells. This content…

5e8a208

… is converted to simple content which is formatted to look like a list using whitespace.

Allow pasting tables with list content inside the cells. This content…

aa6ce58

… is converted to simple content which is formatted to look like a list using whitespace.

mpkelly requested review from gziolo and ajitbohra as code owners December 23, 2022 19:13

Merge branch 'trunk' into add/support-for-pasting-lists-into-tables

38218df

danielbachhuber assigned mpkelly Jan 3, 2023

danielbachhuber self-requested a review January 3, 2023 14:18

danielbachhuber reviewed Jan 3, 2023

View reviewed changes

danielbachhuber added [Feature] Paste [Block] Table Affects the Table Block labels Jan 3, 2023

danielbachhuber requested a review from ellatrix January 9, 2023 23:12

ellatrix reviewed Jan 25, 2023

View reviewed changes

t-hamano mentioned this pull request Feb 24, 2023

Raw Handling: unexpected results when pasting table with list from Google Docs #45774

Open

ellatrix requested changes Oct 9, 2023

View reviewed changes

mpkelly mentioned this pull request Oct 26, 2023

Paste Handler: convert lists inside of table cells to pseudo lists WIP #55651

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow pasting tables with list content inside the cells. #46775

Allow pasting tables with list content inside the cells. #46775

mpkelly commented Dec 23, 2022 •

edited by ellatrix

Loading

mpkelly commented Dec 23, 2022

mpkelly commented Dec 23, 2022

danielbachhuber commented Jan 3, 2023

danielbachhuber left a comment •

edited

Loading

danielbachhuber commented Jan 5, 2023

mpkelly commented Jan 11, 2023 •

edited

Loading

danielbachhuber commented Jan 11, 2023

mpkelly commented Jan 11, 2023 •

edited

Loading

annezazu commented Jan 11, 2023

ellatrix Jan 25, 2023

ellatrix Jan 25, 2023

ellatrix Jan 25, 2023

ellatrix Jan 25, 2023

ellatrix left a comment

jordesign commented Jul 28, 2023

ellatrix left a comment

mpkelly commented Oct 10, 2023

ellatrix commented Oct 10, 2023

mpkelly commented Oct 26, 2023

	{
	type: 'raw',
	schema: () => ( {
	blockquote: {
	children: '*',
	},
	} ),
	selector: 'blockquote',
	transform: ( node, handler ) => {
	return createBlock(
	'core/quote',
	// Don't try to parse any `cite` out of this content.
	// * There may be more than one cite.
	// * There may be more attribution text than just the cite.
	// * If the cite is nested in the quoted text, it's wrong to
	// remove it.
	{},
	handler( {
	HTML: node.innerHTML,
	mode: 'BLOCKS',
	} )
	);
	},
	},

	children: getListContentSchema( { phrasingContentSchema } ),
	children: '*',

Allow pasting tables with list content inside the cells. #46775

Are you sure you want to change the base?

Allow pasting tables with list content inside the cells. #46775

Conversation

mpkelly commented Dec 23, 2022 • edited by ellatrix Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

mpkelly commented Dec 23, 2022

Example 1

Google Docs

Paste Into Gutenberg

Example 2

Google Docs

Gutenberg Paste

Example 3

Google Docs

Paste Into Gutenberg

Example 4

Google Docs

Paste Into Gutenberg

Example 5

Google Docs

Paste Into Gutenberg

mpkelly commented Dec 23, 2022

danielbachhuber commented Jan 3, 2023

danielbachhuber left a comment • edited Loading

Choose a reason for hiding this comment

danielbachhuber commented Jan 5, 2023

mpkelly commented Jan 11, 2023 • edited Loading

danielbachhuber commented Jan 11, 2023

mpkelly commented Jan 11, 2023 • edited Loading

annezazu commented Jan 11, 2023

ellatrix Jan 25, 2023

Choose a reason for hiding this comment

ellatrix Jan 25, 2023

Choose a reason for hiding this comment

ellatrix Jan 25, 2023

Choose a reason for hiding this comment

ellatrix Jan 25, 2023

Choose a reason for hiding this comment

ellatrix left a comment

Choose a reason for hiding this comment

jordesign commented Jul 28, 2023

ellatrix left a comment

Choose a reason for hiding this comment

mpkelly commented Oct 10, 2023

ellatrix commented Oct 10, 2023

mpkelly commented Oct 26, 2023

mpkelly commented Dec 23, 2022 •

edited by ellatrix

Loading

danielbachhuber left a comment •

edited

Loading

mpkelly commented Jan 11, 2023 •

edited

Loading

mpkelly commented Jan 11, 2023 •

edited

Loading