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.

Sign up for GitHub

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

Jump to bottom

Add table block caption #15554

Merged
merged 11 commits into from
Dec 2, 2019
Merged

Add table block caption #15554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9aaa961
Initial attempt at table block caption
talldan May 9, 2019
3e05aff
Reuse the captionId when already set as an attribute
talldan Aug 28, 2019
1793afb
Change style rule to use figcaption selector
talldan Aug 28, 2019
8f5d4ec
Explain use of aria-labelledby
talldan Aug 28, 2019
65e2063
Update deprecation
talldan Aug 28, 2019
91c3e80
Use RichText for figcaption, strip out a11y hacks
talldan Oct 18, 2019
0265104
Ensure empty classname is not output onto table element and update sn…
talldan Oct 21, 2019
f3ae751
Update raw handling test
talldan Oct 21, 2019
a0adac7
Revert "Update raw handling test"
talldan Dec 2, 2019
9181248
Revert "Ensure empty classname is not output onto table element and u…
talldan Dec 2, 2019
6474a17
Update table caption fixtures and deprecations
talldan Dec 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/block-library/src/table/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
"backgroundColor": {
"type": "string"
},
"caption": {
"type": "string",
"source": "html",
"selector": "figcaption",
"default": ""
},
"head": {
"type": "array",
"default": [],
Expand Down
105 changes: 99 additions & 6 deletions packages/block-library/src/table/deprecated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,111 @@ import classnames from 'classnames';
*/
import { RichText, getColorClassName } from '@wordpress/block-editor';

/**
* Internal dependencies
*/
import metadata from './block.json';

const supports = {
align: true,
};

const deprecated = [
{
attributes: metadata.attributes,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to filter out the new attributes, instead of duplicating the whole object?

Copy link
Contributor Author

@talldan talldan Aug 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of that approach as it seems quite error prone for someone adding a new deprecation or attribute. Once these attributes are defined for a deprecation ideally they won't ever change.

attributes: {
hasFixedLayout: {
type: 'boolean',
default: false,
},
backgroundColor: {
type: 'string',
},
head: {
type: 'array',
default: [],
source: 'query',
selector: 'thead tr',
query: {
cells: {
type: 'array',
default: [],
source: 'query',
selector: 'td,th',
query: {
content: {
type: 'string',
source: 'html',
},
tag: {
type: 'string',
default: 'td',
source: 'tag',
},
scope: {
type: 'string',
source: 'attribute',
attribute: 'scope',
},
},
},
},
},
body: {
type: 'array',
default: [],
source: 'query',
selector: 'tbody tr',
query: {
cells: {
type: 'array',
default: [],
source: 'query',
selector: 'td,th',
query: {
content: {
type: 'string',
source: 'html',
},
tag: {
type: 'string',
default: 'td',
source: 'tag',
},
scope: {
type: 'string',
source: 'attribute',
attribute: 'scope',
},
},
},
},
},
foot: {
type: 'array',
default: [],
source: 'query',
selector: 'tfoot tr',
query: {
cells: {
type: 'array',
default: [],
source: 'query',
selector: 'td,th',
query: {
content: {
type: 'string',
source: 'html',
},
tag: {
type: 'string',
default: 'td',
source: 'tag',
},
scope: {
type: 'string',
source: 'attribute',
attribute: 'scope',
},
},
},
},
},
},
supports,
save( { attributes } ) {
const {
Expand Down
17 changes: 16 additions & 1 deletion packages/block-library/src/table/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,9 +483,16 @@ export class TableEdit extends Component {
className,
backgroundColor,
setBackgroundColor,
setAttributes,
} = this.props;
const { initialRowCount, initialColumnCount } = this.state;
const { hasFixedLayout, head, body, foot } = attributes;
const {
hasFixedLayout,
caption,
head,
body,
foot,
} = attributes;
const isEmpty = isEmptyTableSection( head ) && isEmptyTableSection( body ) && isEmptyTableSection( foot );
const Section = this.renderSection;

Expand Down Expand Up @@ -582,6 +589,14 @@ export class TableEdit extends Component {
<Section name="body" rows={ body } />
<Section name="foot" rows={ foot } />
</table>
<RichText
tagName="figcaption"
placeholder={ __( 'Write caption…' ) }
value={ caption }
onChange={ ( value ) => setAttributes( { caption: value } ) }
// Deselect the selected table cell when the caption is focused.
unstableOnFocus={ () => this.setState( { selectedCell: null } ) }
/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like this hack with the separate figcaption and RichText elements. I'm not sure if this was a known issue in the image block and accepted that it wouldn't work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not wrap the <RichText> instance in a <figure> element, and use a div as the tagName? One day we'll no longer have all the wrapping tags, but until that day we could do it like that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One day we'll no longer have all the wrapping tags

Would love you to have a look at #17607, which will allow the table block to omit the wrapper div.

</figure>
</>
);
Expand Down
6 changes: 6 additions & 0 deletions packages/block-library/src/table/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,17 @@
text-align: left;
align-items: center;
}

&__placeholder-input {
width: 100px;
}

&__placeholder-button {
min-width: 100px;
justify-content: center;
}

figcaption {
@include caption-style-theme();
}
}
9 changes: 9 additions & 0 deletions packages/block-library/src/table/save.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default function save( { attributes } ) {
body,
foot,
backgroundColor,
caption,
} = attributes;
const isEmpty = ! head.length && ! body.length && ! foot.length;

Expand All @@ -29,6 +30,8 @@ export default function save( { attributes } ) {
'has-background': !! backgroundClass,
} );

const hasCaption = ! RichText.isEmpty( caption );

const Section = ( { type, rows } ) => {
if ( ! rows.length ) {
return null;
Expand Down Expand Up @@ -69,6 +72,12 @@ export default function save( { attributes } ) {
<Section type="body" rows={ body } />
<Section type="foot" rows={ foot } />
</table>
{ hasCaption && (
<RichText.Content
tagName="figcaption"
value={ caption }
/>
) }
</figure>
);
}
4 changes: 4 additions & 0 deletions packages/block-library/src/table/theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,8 @@
border: 1px solid;
word-break: normal;
}

figcaption {
@include caption-style-theme();
}
}
6 changes: 6 additions & 0 deletions packages/e2e-tests/fixtures/block-transforms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,12 @@ export const EXPECTED_TRANSFORMS = {
'Group',
],
},
core__table__caption: {
originalBlock: 'Table',
availableTransforms: [
'Group',
],
},
'core__table__scope-attribute': {
originalBlock: 'Table',
availableTransforms: [
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-tests/fixtures/blocks/core__table.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"isValid": true,
"attributes": {
"hasFixedLayout": false,
"caption": "",
"head": [
{
"cells": [
Expand Down
3 changes: 3 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__table__caption.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!-- wp:core/table -->
<figure class="wp-block-table"><table class=""><thead><tr><th>Version</th><th>Musician</th><th>Date</th></tr></thead><tbody><tr><td><a href="https://wordpress.org/news/2003/05/wordpress-now-available/">.70</a></td><td>No musician chosen.</td><td>May 27, 2003</td></tr><tr><td><a href="https://wordpress.org/news/2004/01/wordpress-10/">1.0</a></td><td>Miles Davis</td><td>January 3, 2004</td></tr><tr><td>Lots of versions skipped, see <a href="https://codex.wordpress.org/WordPress_Versions">the full list</a></td><td>&hellip;</td><td>&hellip;</td></tr><tr><td><a href="https://wordpress.org/news/2015/12/clifford/">4.4</a></td><td>Clifford Brown</td><td>December 8, 2015</td></tr><tr><td><a href="https://wordpress.org/news/2016/04/coleman/">4.5</a></td><td>Coleman Hawkins</td><td>April 12, 2016</td></tr><tr><td><a href="https://wordpress.org/news/2016/08/pepper/">4.6</a></td><td>Pepper Adams</td><td>August 16, 2016</td></tr><tr><td><a href="https://wordpress.org/news/2016/12/vaughan/">4.7</a></td><td>Sarah Vaughan</td><td>December 6, 2016</td></tr></tbody></table><figcaption>A table for testing</figcaption></figure>
<!-- /wp:core/table -->
146 changes: 146 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__table__caption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
[
{
"clientId": "_clientId_0",
"name": "core/table",
"isValid": true,
"attributes": {
"hasFixedLayout": false,
"caption": "A table for testing",
"head": [
{
"cells": [
{
"content": "Version",
"tag": "th"
},
{
"content": "Musician",
"tag": "th"
},
{
"content": "Date",
"tag": "th"
}
]
}
],
"body": [
{
"cells": [
{
"content": "<a href=\"https://wordpress.org/news/2003/05/wordpress-now-available/\">.70</a>",
"tag": "td"
},
{
"content": "No musician chosen.",
"tag": "td"
},
{
"content": "May 27, 2003",
"tag": "td"
}
]
},
{
"cells": [
{
"content": "<a href=\"https://wordpress.org/news/2004/01/wordpress-10/\">1.0</a>",
"tag": "td"
},
{
"content": "Miles Davis",
"tag": "td"
},
{
"content": "January 3, 2004",
"tag": "td"
}
]
},
{
"cells": [
{
"content": "Lots of versions skipped, see <a href=\"https://codex.wordpress.org/WordPress_Versions\">the full list</a>",
"tag": "td"
},
{
"content": "…",
"tag": "td"
},
{
"content": "…",
"tag": "td"
}
]
},
{
"cells": [
{
"content": "<a href=\"https://wordpress.org/news/2015/12/clifford/\">4.4</a>",
"tag": "td"
},
{
"content": "Clifford Brown",
"tag": "td"
},
{
"content": "December 8, 2015",
"tag": "td"
}
]
},
{
"cells": [
{
"content": "<a href=\"https://wordpress.org/news/2016/04/coleman/\">4.5</a>",
"tag": "td"
},
{
"content": "Coleman Hawkins",
"tag": "td"
},
{
"content": "April 12, 2016",
"tag": "td"
}
]
},
{
"cells": [
{
"content": "<a href=\"https://wordpress.org/news/2016/08/pepper/\">4.6</a>",
"tag": "td"
},
{
"content": "Pepper Adams",
"tag": "td"
},
{
"content": "August 16, 2016",
"tag": "td"
}
]
},
{
"cells": [
{
"content": "<a href=\"https://wordpress.org/news/2016/12/vaughan/\">4.7</a>",
"tag": "td"
},
{
"content": "Sarah Vaughan",
"tag": "td"
},
{
"content": "December 6, 2016",
"tag": "td"
}
]
}
],
"foot": []
},
"innerBlocks": [],
"originalContent": "<figure class=\"wp-block-table\"><table class=\"\"><thead><tr><th>Version</th><th>Musician</th><th>Date</th></tr></thead><tbody><tr><td><a href=\"https://wordpress.org/news/2003/05/wordpress-now-available/\">.70</a></td><td>No musician chosen.</td><td>May 27, 2003</td></tr><tr><td><a href=\"https://wordpress.org/news/2004/01/wordpress-10/\">1.0</a></td><td>Miles Davis</td><td>January 3, 2004</td></tr><tr><td>Lots of versions skipped, see <a href=\"https://codex.wordpress.org/WordPress_Versions\">the full list</a></td><td>&hellip;</td><td>&hellip;</td></tr><tr><td><a href=\"https://wordpress.org/news/2015/12/clifford/\">4.4</a></td><td>Clifford Brown</td><td>December 8, 2015</td></tr><tr><td><a href=\"https://wordpress.org/news/2016/04/coleman/\">4.5</a></td><td>Coleman Hawkins</td><td>April 12, 2016</td></tr><tr><td><a href=\"https://wordpress.org/news/2016/08/pepper/\">4.6</a></td><td>Pepper Adams</td><td>August 16, 2016</td></tr><tr><td><a href=\"https://wordpress.org/news/2016/12/vaughan/\">4.7</a></td><td>Sarah Vaughan</td><td>December 6, 2016</td></tr></tbody></table><figcaption>A table for testing</figcaption></figure>"
}
]
Loading