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

Table Block: Should we use InnerBlocks? #18768

Open
Tracked by #32400
melchoyce opened this issue Nov 26, 2019 · 25 comments
Open
Tracked by #32400

Table Block: Should we use InnerBlocks? #18768

melchoyce opened this issue Nov 26, 2019 · 25 comments
Labels
[Block] Table Affects the Table Block [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Type] Question Questions about the design or development of the editor.

Comments

@melchoyce
Copy link
Contributor

After using Media & Text, Cover, Columns, etc., it feels weird to use the Table block and have it not somehow use InnerBlocks — I keep expecting individual cell options, or row-column options, and the ability to easily move cells around using block movers.

@melchoyce melchoyce added [Type] Question Questions about the design or development of the editor. [Block] Table Affects the Table Block labels Nov 26, 2019
@ZebulanStanphill
Copy link
Member

How exactly would moving a cell work? Would it swap places with another cell? How would you handle moving rows or columns, if you handle that at all? If there were row blocks, then there couldn't be column blocks, and vice-versa.

@phpbits
Copy link
Contributor

phpbits commented Nov 27, 2019

I agree with this, specially with adding buttons inside the table.

@jasmussen
Copy link
Contributor

For what it's worth, @ellatrix made this happen in a pull request a while back, and what we found trying to style the editor version to match the front-end was that it was extremely difficult due to the extra divs and elements that are necessary to make editable fields in the block editor. At the time we decided not to pursue it until we can reliably use something like display: contents;. Those "caniuse" stats look better, but require us to shed support for IE11 and to an extent, even classic Edge (the new Chromium based Edge is fine: https://www.microsoftedgeinsider.com/en-us/). Additional refactorings, such as improvements to horizontal margins, have happened since that conversation, so it's looking more realistic now than it did then. But it's still going to be a challenge!

@ellatrix
Copy link
Member

ellatrix commented Nov 27, 2019

I've refactored RichText so it doesn't have any wrapper div elements left, but blocks still have a lot of wrapper div element for all the controls and positioning. This will make it much harder to get to work. I kind of wish we never put block controls in the block DOM, but rather put it outside the block list and position it correctly when a block is selected.

@ZebulanStanphill
Copy link
Member

@ellatrix I agree that moving the block toolbar outside of the block DOM would be a good idea. Is it too late to change the implementation now? I think you could do it with something like React Portals?

@ellatrix
Copy link
Member

@ZebulanStanphill No, definitely not too late. With some adjustments to Popover, it shouldn't be too hard to do. I started work in #18779.

@talldan
Copy link
Contributor

talldan commented Dec 2, 2019

I've thought a bit about how this might work.

First off, there would probably have to be inner blocks for table headers (<thead>), bodies (<tbody>) and footers (<tfoot>) so that they can be added/removed. Maybe a 'Table Section' block. The top-most table block could render these Table Sections as inner blocks.

Each of those Table Sections could render inner Table Row blocks (<tr>). That gives the handy functionality of being able to select rows as blocks add new rows using the block inserter.

The tricky part is columns. A column is individual <td> or <th> elements that might span across several rows and sections. Not really sure how those could be modelled as blocks or how selection/insertion of a column might work.

Maybe someone has a good idea how that might be possible.

@strarsis
Copy link
Contributor

It would be great to have table row blocks and cell blocks, maybe even "virtual" column blocks.
Then each cell and row can be block styles assigned. This is very useful! And it is possible in HTML, too.

@talldan
Copy link
Contributor

talldan commented Oct 23, 2020

I wonder if the new useInnerBlockProps React hook helps with the technical challenges on this issue now. Or alternatively offers possibilities for a new API for declaring tables.

Now that the children inner blocks can be accessed, potentially the array could be mapped into table sections, rows and cells.

A wrapper around useInnerBlockProps that handles the mapping could be one way to expose this:

	const { sections } = useInnerBlockTable(
		{},
		{ sections: [ 'thead', 'tbody', 'tfoot' ], rows: 4, columns: 4 }
	);

	return (
		<table>
			{ sections.map( ( { name: Tag, props: sectionProps, rows } ) => (
				<Tag { ...sectionProps }>
					{ rows.map( ( { props: rowProps, cells }, index ) => (
						<tr { ...rowProps }>
							{ cells.map( ( { cell } ) => cell ) }
						</tr>
					) ) }
				</Tag>
			) ) }
		</table>
	);

I think the main challenge would still be selecting blocks—the block editor doesn't support row or column selection, only sequential selection. 🤔

@jasmussen
Copy link
Contributor

The lighter DOM prop will help the table a lot. But this is still the primary blocker:

I think the main challenge would still be selecting blocks—the block editor doesn't support row or column selection, only sequential selection. 🤔

Table is almost a textbook example for why we might need the "passthrough" prop from #7694. In the following example markup:

<table>
    <thead>
        <tr>
            <th colspan="2">The table header</th>
        </tr>
    </thead>
    <tbody>
        <tr>
            <td>The table body</td>
            <td>with two columns</td>
        </tr>
    </tbody>
</table>

For a good user experience you should never have to select anything other than the table itself and the td cells inside. Every other nesting elements should simply not be selectable. If the passthrough prop can help accomplish that, I think we'll be ready to move to nesting!

@talldan
Copy link
Contributor

talldan commented Nov 18, 2020

For a good user experience you should never have to select anything other than the table itself and the td cells inside. Every other nesting elements should simply not be selectable. If the passthrough prop can help accomplish that, I think we'll be ready to move to nesting!

I was thinking that there'd be no need for passthrough blocks (for table sections/rows). My idea is there'd be two blocks core/table and core/table-cell, with the latter being the inner block.

With useInnerBlocksProps we have access to the individual inner blocks. All the inner 'core/table-cell' blocks end up in a single array because that's how the block editor stores them:

const { children } = useInnerBlocksProps();
// inner blocks for the table block are table cell blocks like this:
// [ 
//	 <TableCell />, 
//	 <TableCell />, 
//	 <TableCell />,
//	 <TableCell />,
//       ...
// ]

Now that we have access to that array (thanks to useInnerBlocksProps), they can be mapped into table sections and rows:

const rows = mapToRows( children, rowSizes );
// The previous single array of cells is now mapped into rows:
// [
//       row 1:
//	 [ 
//                <TableCell />, 
//        	  <TableCell />,
//       ],
//       row 2:
//	 [ 
//                <TableCell />, 
//        	  <TableCell />,
//       ],
//       ...
// ]

Then elements like tbody and tr are just rendered as normal elements with the table cell blocks inside them.

The issue I mentioned with selection is not related to clicking through the block heirarchy, but multi-block selection. Not really an issue for rows which is just selecting consecutive blocks and already supported, but column selections would be a non-contiguous block selection (as described in these issues: #2320, #16895), as the selection would be every nth inner block element.

That would still be a beneficial feature for the block editor generally, but something that's been tricky to implement before.

Other challenges:

  • We'd have to keep a data representation of the sections, number of rows in each section, and cells in each row somewhere, and it would have to stay in sync with the blocks.
  • Similar to selection, inserting a column would also be inserting every nth block (which is not very nice technically, but possible).
  • While we have useInnerBlocksProps for a block's edit, I don't think we have the same for save.

@jasmussen
Copy link
Contributor

Awesome approach, and thank you for outlining it so clearly.

That would still be a beneficial feature for the block editor generally, but something that's been tricky to implement before.

I recall conversations about non-contiguous multi selections in the past. Would a good first step be to allow you to select multiple separate cells by holding ⌘ when you click?

@paaljoachim
Copy link
Contributor

Can we get a status update?
As it would be good to know where this issue is at and how we can move forward.
Thanks!

@paaljoachim paaljoachim added the [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P label Jan 12, 2021
@joelyoder
Copy link

I would love to help this move forward however I can, since we have some needs for this functionality, especially the ability to merge cells. I'm not sure how much I would be able to offer on the editor dev side of things, but I'd be happy to do some testing.

@fabiankaegy
Copy link
Member

Same here :) would love to help move this forward. Merging cells and having some more flexibility in tables is something that I keep running into.

And the approach by @talldan seems very feasible.

@talldan do you have any particular blockers that came out of your discovery or is this just a lack of time? :)

Thanks in advance! :)

@talldan
Copy link
Contributor

talldan commented Apr 7, 2021

I definitely don't have time at the moment, unfortunately.

But I think a good first step would be to bring more awareness to this issue. I've outlined a potential option, but always good to get a few more ideas and critique.

There are also things to be learnt from the similar change for the gallery block about how we might approach such a transition.

@jasmussen
Copy link
Contributor

For the approach to the table block, I would say it is important that the highly nested HTML markup a table comes with, does not dictate the user experience; the nesting UI would get unwieldy quick. In that vein, it might be good to revisit #7694 before going too far down the path.

@talldan
Copy link
Contributor

talldan commented Apr 7, 2021

@jasmussen I agree, I think we had a similar discussion already above. My idea was to only have two block types, table and table cell. I think the other parts (sections, columns, rows) could be treated as multi-cell selections, so #7694 wouldn't be required.

I definitely don't think it makes sense to pursue column/row blocks, not even sure how that could work.

@jasmussen
Copy link
Contributor

Apologies, I'm tired these days 🙈 — yes, table and table cell makes sense.

@ramonjd
Copy link
Member

ramonjd commented Dec 15, 2021

I stumbled across this issue as block cell selection/merging was something I am missing too! 😄

I was wondering, would we need to migrate to inner blocks before we look at the selection/merge cell functionality?

A script that tracks and manages a table matrix (cell states, merges etc), along with DOM events to detect selected cells, might be useful to look at first.

A "cell" today is a td or th. Tomorrow it might be <TableCell rowspan="2" />.

I was thinking so long as our script in the Table Block can discern cell metadata (be it a regular td or wrapped inner block) from the event.target, would it matter if a table in the editor is made up of inner blocks or not?

Kapture 2021-12-16 at 08 52 25

@jasmussen
Copy link
Contributor

would it matter if a table in the editor is made up of inner blocks or not?

Last we experimented with this, the challenge was primarily the amount of extra markup that the editor creates when nesting is involved. Since then the situation here has vastly improved with the "lighter DOM" explorations, possibly making it more feasible. So as a rule of thumb, I'd say that if the elements in the editor match the elements on the frontend (give or take a few classes), stylewise we should be in a good place.

@talldan
Copy link
Contributor

talldan commented Jan 10, 2022

It is going to be a big leap to migrate the table block over to inner blocks, and it probably needs more discussion on whether there's value in proceeding. Doing so would probably need one or two dedicated developers.

There's still some distance between how inner blocks work currently, and how a table might use inner blocks. I think the big challenge is around how inner blocks currently only have a single dimension (they're a 1 dimensional list), while tables are two dimensional (with columns and rows). I think some ideas and proof of concepts for that are needed.

Using inner blocks would bring some advantages because blocks already support some functionality that a table does (insertion, deletion, reordering, multi-selection, merging). Using those existing features would be pretty elegant, and leaning into the block system means that improvements to the table block benefit more than just the table block.

But I can definitely see how challenging it is, and that there isn't a very clear path forwards. I think some of these features (merging in particular) will be very challenging anyway!

@ellatrix
Copy link
Member

Yes, handling two dimensions will be tough, especially with the extra tr markup. With the row element, moving rows is automatically built in, but there's no equivalent for columns (no table column element, which wouldn't be possible to have anyway).

@CarlyOvers
Copy link

+1 would love this feature

@paaljoachim
Copy link
Contributor

Can the grid feature being worked on #57478 also be used in a similar way for the Table block?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Table Affects the Table Block [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

No branches or pull requests