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

Add Text Columns → Columns transform #9364

Merged
merged 2 commits into from
Aug 27, 2018
Merged

Add Text Columns → Columns transform #9364

merged 2 commits into from
Aug 27, 2018

Conversation

ZebulanStanphill
Copy link
Member

Description

This PR adds a one-way Text Columns → Columns transform to make it easier to migrate from the deprecated block. The alignment, custom CSS classes, and content are all preserved.

How has this been tested?

You can test this branch by copy-pasting this markup into a post and trying out the transforms on the Text Columns blocks:

<!-- wp:text-columns {"className":"yoyo"} -->
<div class="wp-block-text-columns alignundefined columns-2 yoyo"><div class="wp-block-column"><p>Content<br/>2nd line in same paragraph<br/><br/>4th line<br/></p></div><div class="wp-block-column"><p>More content</p></div></div>
<!-- /wp:text-columns -->

<!-- wp:text-columns {"width":"center","className":"donut"} -->
<div class="wp-block-text-columns aligncenter columns-2 donut"><div class="wp-block-column"><p>Content<br/>2nd line in same paragraph<br/><br/>4th line<br/></p></div><div class="wp-block-column"><p>More content</p></div></div>
<!-- /wp:text-columns -->

<!-- wp:text-columns {"width":"wide","className":"neato"} -->
<div class="wp-block-text-columns alignwide columns-2 neato"><div class="wp-block-column"><p>Content<br/>2nd line in same paragraph<br/><br/>4th line<br/></p></div><div class="wp-block-column"><p>More content</p></div></div>
<!-- /wp:text-columns -->

Additional notes

Ideally, you would want to automatically transform deprecated blocks into the recommended replacements, but the deprecated blocks API does not currently support migrating from/to a completely different block, so using the transforms API to allow for easy user-initiated transforms is currently the best option.

Checklist:

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

Related issues and PRs

@chrisvanpatten
Copy link
Contributor

Ideally, you would want to automatically transform deprecated blocks into the recommended replacements, but the deprecated blocks API does not currently support migrating from/to a completely different block, so using the transforms API to allow for easy user-initiated transforms is currently the best option.

I don't want to derail this PR, but I'm super curious if this has been discussed in the past. This feels like a great opportunity to improve the deprecation experience.

@ZebulanStanphill
Copy link
Member Author

@chrisvanpatten If it has been discussed before, I don't know about it. 🤷‍♂️ I agree that it would be great to have that kind of functionality, and I'm also curious if it has been discussed before.

align: [ 'wide', 'full' ],
supports,

transforms: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the declaration of this transform the the text-columns block instead (to transform) that way when/if we remove the block, we don't forget the transform.

@ZebulanStanphill
Copy link
Member Author

@youknowriad Addressed feedback; the transform is now defined in text-columns/index.js.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks

@youknowriad youknowriad merged commit 349e892 into WordPress:master Aug 27, 2018
@ZebulanStanphill ZebulanStanphill deleted the update/add-text-columns-to-columns-transform branch August 27, 2018 15:04
@ZebulanStanphill
Copy link
Member Author

@youknowriad This should be added to the 3.7 milestone.

@youknowriad youknowriad added this to the 3.7 milestone Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants