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

Support colspan attribute in table HTML, including when pasting #45981

Merged

Conversation

mpkelly
Copy link
Contributor

@mpkelly mpkelly commented Nov 22, 2022

What?

Support colspan attribute on th and td when pasting tables in the Editor.

Why?

It was based on this requirement in #45774

How?

Testing Instructions

  1. Check out this branch and try pasting tables in from external sources. The colspan attribute should be supported on th and td elements within thead, tbody and tfoot elements.

Screenshots or screencast

image

@codesandbox
Copy link

codesandbox bot commented Nov 22, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 22, 2022
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @mpkelly! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @mpkelly !

The table seems to be formatted correctly on paste:

image

However, I don't see any colspan="" on the serialized data...

image

Then, when I refresh, the formatting is lost:

image

Maybe packages/block-library/src/table/save.js needs to handle colspan? It'd probably be good to have update the tests for this particular scenario, too.

@mpkelly
Copy link
Contributor Author

mpkelly commented Nov 29, 2022

Thanks for the review, @danielbachhuber.

I will try and get this working now. I would also like to look at the list transformation you mentioned too.

@mpkelly
Copy link
Contributor Author

mpkelly commented Nov 29, 2022

It's now saving for me, and I can view it in a preview. I tried to add tests - just some snapshot tests - but it was too difficult. These simple components implicitly reference all kinds of stuff, which is likely why we don't have tests for the same functions in other blocks.

@mpkelly mpkelly requested review from danielbachhuber and removed request for ajitbohra November 29, 2022 19:50
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Works great in my testing!

The <thead> and <tfoot> persist through save:

image

</tbody>
</table>
</div>
<br/><br/></b>
Copy link
Member

Choose a reason for hiding this comment

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

Minor: this maybe makes more sense to add to the integration tests, which has all blocks loaded.

Copy link
Member

Choose a reason for hiding this comment

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

@ellatrix Would it make sense to keep this existing test and add it to integration tests?

Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Looks good to me! Will be interesting to add some UI for this. 😄

@danielbachhuber danielbachhuber added [Block] Table Affects the Table Block [Type] Feature New feature to highlight in changelogs. labels Nov 30, 2022
@danielbachhuber danielbachhuber changed the title Support colspan attribute on tables when pasting Support colspan attribute in table HTML, including when pasting Nov 30, 2022
@danielbachhuber danielbachhuber merged commit 796b800 into WordPress:trunk Nov 30, 2022
@github-actions
Copy link

Congratulations on your first merged pull request, @mpkelly! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 14.7 milestone Nov 30, 2022
@mpkelly
Copy link
Contributor Author

mpkelly commented Nov 30, 2022

Thanks for the reviews and merge! I will look at moving the test in another PR.

@DaisyOlsen DaisyOlsen added [Type] Enhancement A suggestion for improvement. and removed [Type] Feature New feature to highlight in changelogs. labels Nov 30, 2022
mpkelly added a commit to mpkelly/gutenberg that referenced this pull request Dec 7, 2022
…ordPress#45981)

Co-authored-by: Daniel Bachhuber <daniel.bachhuber@automattic.com>
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 First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants