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

Remove Text Columns block from insertion menus #8036

Merged
merged 2 commits into from
Jul 19, 2018

Conversation

brandonpayton
Copy link
Member

@brandonpayton brandonpayton commented Jul 18, 2018

Description

This is a simple PR to remove the Text Columns block from the block library and other insertion menus. It also prints a deprecation warning when loading a Text Columns block in the editor.

Closes #6506.
NOTE: It doesn't do everything called for in #6506 but follows Matías' direction on the issue and addresses the concern for the Try Gutenberg callout.

How has this been tested?

  • Ran unit and e2e tests.
  • Loaded the editor and verified that the core/text-columns block is not present in the block library or the block autocompletion menu.
  • Loaded a post with a pre-existing Text Columns block and observed that the block was loaded as a Text Columns block and editable.

Checklist:

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

@brandonpayton brandonpayton added the [Feature] Blocks Overall functionality of blocks label Jul 18, 2018
@brandonpayton brandonpayton added this to the Try Callout milestone Jul 18, 2018
@brandonpayton brandonpayton self-assigned this Jul 18, 2018
@brandonpayton brandonpayton requested a review from noisysocks July 18, 2018 20:23
@brandonpayton brandonpayton removed this from the Try Callout milestone Jul 18, 2018
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

It works! 🐩

Should we remove the (beta) from Columns (beta)?

@@ -25,6 +26,9 @@ import './editor.scss';
export const name = 'core/text-columns';

export const settings = {
// Use an empty parent array to indicate this block should no longer be available for insertion.
parent: [],
Copy link
Member

Choose a reason for hiding this comment

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

A more "official" way of doing this is to use supports.inserter: false. Up to you which way you go—they should be functionally equivalent 🙂

supports: {
customClassName: false,
html: false,
inserter: false,
},

Copy link
Member

Choose a reason for hiding this comment

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

Let's follow what we have for Shared block 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

That's much more explicit. I'll switch to that. Thanks for letting me know.

@@ -64,6 +68,12 @@ export const settings = {
edit: ( ( { attributes, setAttributes, className } ) => {
const { width, content, columns } = attributes;

deprecated( 'The Text Columns block', {
version: 'a future version',
Copy link
Member

Choose a reason for hiding this comment

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

version is an optional param, so we could just omit it entirely instead of being vague here.

Copy link
Member

@noisysocks noisysocks Jul 19, 2018

Choose a reason for hiding this comment

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

Oh—and we should update deprecated.md.

@ZebulanStanphill
Copy link
Member

@noisysocks I would wait until a basic solution for #6048 is implemented before removing the beta label from the Columns block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants