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

Layout: fix flex direction column #42939

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Conversation

matiasbenedetto
Copy link
Contributor

@matiasbenedetto matiasbenedetto commented Aug 3, 2022

Fixes: #42938

What?

Fixing stacked (column/vertical) orientation of the row block.

Why?

The row element with stack orientation doesn't work since #42452 was merged.
Currently, the CSS related to the flex-direction is not rendered in the frontend.

How?

This PR fixes the styles engine to render the CSS to the flex-direction in the frontend.

Testing Instructions

  • Add a Row element
  • Add two paragraphs inside the row element
  • Select stack orientation in the row element
  • Save the post and verify in the frontend that the elements are not vertically stacked.

Screenshots or screencast

Before:
image

After:
image

Fixes: #42938

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

Thanks for catching that one. Must have slipped past us 😄

'align-items' => 'flex-start',
$layout_styles[] = array(
'selector' => $selector,
'declarations' => array( 'flex-direction' => 'column' ),
Copy link
Member

Choose a reason for hiding this comment

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

Not sure, but should the align-items prop also come across?

Suggested change
'declarations' => array( 'flex-direction' => 'column' ),
'declarations' => array( 'flex-direction' => 'column', 'align-items' => 'flex-start' ),

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we already have a duplicate 'align-items' => 'flex-start' in the else condition below, so I think we were duplicating that output previously?

@ramonjd ramonjd changed the title Styles engine: fix flex direction column Layout: fix flex direction column Aug 3, 2022
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks so much for catching this one @matiasbenedetto! It turns out with #42452, I was testing the layout implementation using the Buttons block, which provides its own vertical orientation CSS, so it didn't catch this one in manual testing 🤦

We'll now know to properly test the Stack variation, too!

This LGTM, so I'll merge it in once the e2es pass. Thanks! 🙇

@andrewserong
Copy link
Contributor

I've just given this a rebase to get the e2e tests passing now that #42947 has been merged

@andrewserong andrewserong merged commit 88899c8 into trunk Aug 4, 2022
@andrewserong andrewserong deleted the fix/flex-direction-column branch August 4, 2022 02:34
@github-actions github-actions bot added this to the Gutenberg 13.9 milestone Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Row element with stack orientation doesn't work
3 participants