Add table dimension controls

[talldan]

Description

Fixes #9327
Fixes #9114
Fixes #8325
Related #6923

This PR adds width and height controls to the table block. Summarised:

• Add new width/height attributes to the table block
• Add width/height controls to table block sidebar - changing these adjusts the table element's width & height inline styles

How has this been tested?

• Test deprecation by creating a table block on master, saving and loading post on this branch.
• Test pasting a table that has width and height style properties
• Test unsetting table width/height values - table reverts to full width horizontally and content size vertically
• Added unit and integration tests

Screenshots

Using width/height controls

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[talldan]

A couple of things I'm undecided on with this PR:

• Whether I should use a plain text field for width/height values? This would match the mockup, but would lose some of the advantages of the number input. I'm also not sure what's better for accessibility.
• Whether I should just support all css units for width and height, or if it's better to support only a subset (% doesn't really make sense for height)

[talldan]

Hey @jasmussen - This min-width style causes some issues when if I try to set the table width to a value less than it:

gutenberg/packages/block-library/src/table/theme.scss

Line 11 in 5febf46

min-width: $break-mobile / 2;

Do you have any ideas on the right way to resolve that? Thanks!

[jasmussen]

This is very cool! Really nice work.

This min-width style causes some issues when if I try to set the table width to a value less than it:

Sigh, creating responsive tables is a dang headache.

We need some kind of min-width. We could make it optional, like a switch in the inspector. We could even combine the "fixed-width cells" so it's sometning like "Responsive table with fixed-width table cells". Something in that vein.

The other option is to simply accept it as is — what are the use cases for a table that is smaller than 240px?

[afercia]

As a general rule, this way to build translatable strings: ${ __( 'Width' ) } (${ widthUnit }) is not ideal for l10n and it's equivalent to concatenation in PHP. Instead, a better option would be using sprintf() as it would allow translators to swap the order of the words in the string, if need be.

On a side note, I've seen there are other places in the codebase where similar concatenation is used and they all should be reviewed. Best practices for variables in translation strings: https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#variables

[tofumatt]

I would move this ternary out of the component itself and put it in a variable above, to improve readability. Because this line is quite intended, it'll be easier to read above.

As @afercia mentioned, this won't work for l10n because it assumes English order/characters for this string. Instead, you'll want:

import { __, sprintf } from '@wordpress/i18n';

// […]
const widthLabel = widthUnit ? sprintf( __( 'Width (%s)' ), widthUnit ) : __( 'Width' );

<TextControl
  label={ widthLabel }
/>

[talldan]

Thanks both of you for catching that - not sure why my brain blipped on using sprintf. Pushed a commit to fix it.

[jasmussen]

Thanks for working on this. Let me know if you need a hand.

[talldan]

@jasmussen Cheers for the offer - I've decided to move the fix for #9779 to a separate PR (#10011), as this is getting quite big. Would be great if you could take a look at it. 😄

Once that's merged I can squish this down a bit.

[karmatosed]

I think with the comment by @jasmussen we can remove the design feedback label on this as it's in progress. Let's add back in though if we need to.

[gziolo]

This PR became stale, @talldan do you plan to refresh this PR?

[talldan]

@gziolo I'll close this and follow up with a different approach.

Some background—I realised some way through that what I was trying here with the ResizableBox component won't really work with a table. The problem is that ResizableBox outputs a div. If we ever want to be able to resize individual table columns or rows, we won't be able to wrap td elements in a resizablediv as it'll produce invalid markup.

Potential solutions:

• Change ResizableBox so that it can output something other than a div. This could work, but at the moment the table block's td elements are rendered using the RichText component.
• Use a similar approach as the TinyMCE table plugin. The resizable elements are instead rendered in a layer over the top of the table with matching dimensions.