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 vertical center alignment on (mj-)column #231

Closed
janus-reith opened this issue Jun 21, 2020 · 6 comments · Fixed by #235
Closed

Support vertical center alignment on (mj-)column #231

janus-reith opened this issue Jun 21, 2020 · 6 comments · Fixed by #235
Assignees
Labels

Comments

@janus-reith
Copy link

janus-reith commented Jun 21, 2020

Currently, a warning is shown if the verticalAlignmentattribute for blocks of type "core/column" is set to center.

mj-column does support center alignment, but calls it "middle".

In the renderer just check if verticalAlignment equals "center" and subsitute that with "middle" :

$column_attrs['vertical-align'] = $attrs['verticalAlignment'];

Tested it, works fine.

@adekbadek adekbadek self-assigned this Jun 23, 2020
@adekbadek
Copy link
Member

adekbadek commented Jun 23, 2020

Hi @janus-reith, thanks for the tip!
I've found that in multi-column layout, the tallest column has to be vertical-align="middle" in order to make smaller columns handle the middle alignment, does that check out with your testing?
Example: https://mjml.io/try-it-live/rkuSMp10L

@janus-reith
Copy link
Author

janus-reith commented Jun 24, 2020

@adekbadek Yes I can confirm, although this seems to be the general way vertical-align="middle" is interpreted in HTML.
Made a quick example here: https://codesandbox.io/s/clever-star-2zgog?file=/index.html
This is not an issue for gutenberg columns, as these use flexbox and align-self for each column.

So this could indeed lead to problems.
Simply enabling it could lead to unexpected results, on the other hand always showing a warning like align: middle only works if the tallest column has align-middle could really disrupt the user experience, and currently also alter the element position.

Still, to really get some more sophisticated layouts done I consider the ability to center-align elements vital.

I wonder if actually measuring column heights is something that could be considered, to only show a warning if really necessary? - Preferably without needing to change anything in the core column component, or outside newspack-newsletters in general.

Looking at this:

const withUnsupportedFeaturesNotices = createHigherOrderComponent( BlockListBlock => {

It looks like the warnings work by wrapping each individual block, with access to their respective attributes.
Now if somehow the wrapping columns block and each column in there could share context, it should be simple to get this done.
But I did not check further how addFilter works here and which restrictions this might impose, or if there maybe is a gutenberg-included solution for this already, or if there could be a more simple way by directly accessing innerBlocks of the parent columns block.

adekbadek added a commit that referenced this issue Jun 24, 2020
@adekbadek
Copy link
Member

adekbadek commented Jun 24, 2020

This is essentially an issue of translating a flexbox layout to a non-flexbox one, which gets pretty complicated.
On one hand there's that, on another we could support an "all columns middle-aligned" layout only – I've submitted a PR: #235

adekbadek added a commit that referenced this issue Jun 24, 2020
@janus-reith
Copy link
Author

Great, seems to be the most simple solution!

@adekbadek
Copy link
Member

Thanks again for digging into that, @janus-reith !

matticbot pushed a commit that referenced this issue Jun 30, 2020
# [1.3.0](v1.2.0...v1.3.0) (2020-06-30)

### Bug Fixes

* apply custom styles even if styling tab in sidebar is closed ([#240](#240)) ([20b08bc](20b08bc))
* localize date formats and notice text ([0cb82f1](0cb82f1))
* posts-inserter columns warning ([5db6b97](5db6b97))
* store translated success note as prop ([5638aee](5638aee))

### Features

* add "sent" status to posts list in dashboard ([ed369e4](ed369e4))
* add a "sent" editor notice after a campaign has been sent ([188908e](188908e))
* column middle alignment handling ([5f82db4](5f82db4)), closes [#231](#231)
* handle custom styling of layouts ([#238](#238)) ([2ba844d](2ba844d)), closes [#225](#225)
@matticbot
Copy link
Contributor

🎉 This issue has been resolved in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants