-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Support colspan
attribute in table
HTML, including when pasting
#45981
Conversation
|
👋 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. |
There was a problem hiding this 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:
However, I don't see any colspan=""
on the serialized data...
Then, when I refresh, the formatting is lost:
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.
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. |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</tbody> | ||
</table> | ||
</div> | ||
<br/><br/></b> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this 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. 😄
colspan
attribute in table
HTML, including when pasting
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! |
Thanks for the reviews and merge! I will look at moving the test in another PR. |
…ordPress#45981) Co-authored-by: Daniel Bachhuber <daniel.bachhuber@automattic.com>
What?
Support
colspan
attribute onth
andtd
when pasting tables in the Editor.Why?
It was based on this requirement in #45774
How?
Testing Instructions
colspan
attribute should be supported onth
andtd
elements withinthead
,tbody
andtfoot
elements.Screenshots or screencast