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

Issue/5612 - Alignfull class not working on columns #5704

Closed

Conversation

jvisick
Copy link
Contributor

@jvisick jvisick commented Mar 20, 2018

Description

The column block isn't outputting alignment classes to the front-end. I think we can fix that and update the block to the supports align extension in one fell swoop.

Fix for #5612.
Related to #5099. (cc @gziolo )

How Has This Been Tested?

Tested locally by adding a column block to a new page, setting it to alignwide, saving and viewing the output markup on the front-end.

Types of changes

Changes to the columns block index.js file by removing any alignment related code and adding:

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

Bug fix (non-breaking change which fixes an issue)

However, this will cause existing saved column blocks with to throw a validation error since it will now be expecting the class to be in the saved markup. I don't know a way around that?

Checklist:

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

@gziolo
Copy link
Member

gziolo commented Mar 21, 2018

The same changes are included in #5099. We haven’t landed them because we discovered a blocker: #5099 (comment). We want to make sure we find the viable solution which allows to handle attributes properly for all type of blocks. We are leaning towards moving attributes to the server to have only one place to apply all required hooks to make align work. Once it is resolved we can get back to this change.

@skorasaurus
Copy link
Member

Will this be addressed with this PR since #5099 has been closed?

@gziolo
Copy link
Member

gziolo commented Aug 27, 2018

Columns block was deprecated, so I don't think is relevant anymore. See #8036.

@gziolo gziolo closed this Aug 27, 2018
@gziolo gziolo added the [Status] Not Implemented Issue/PR we will (likely) not implement. label Aug 27, 2018
@gziolo
Copy link
Member

gziolo commented Aug 27, 2018

Wait, Text Columns was deprecated :) Anyways, I see that it was implemented for Columns block by @jorgefilipecosta:
https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/columns/index.js#L65-L67

Related PR: #6364.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Not Implemented Issue/PR we will (likely) not implement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants