Skip to content

Flex Layout: Fix incorrect default alignment values for Vertical Alignment Control component #68866

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 30, 2025
Merged

Flex Layout: Fix incorrect default alignment values for Vertical Alignment Control component #68866

merged 2 commits into from
May 30, 2025

Conversation

@yogeshbhutkar
Copy link
Contributor

@yogeshbhutkar yogeshbhutkar commented Jan 24, 2025 •
edited
Loading

What, Why and How?

Fixes: #68864

This PR addresses a bug in which the default vertical alignment value does not render as selected in the Vertical Alignment Toolbar when the orientation is set to vertical. The issue arises because the layout.verticalAlignment property is expected to store the alignment key (e.g., top, center, bottom, stretch, etc.) but instead holds the resolved CSS value (e.g., flex-start).

Testing Instructions

  1. Create a Row Block.
  2. Verify from the Block Toolbar that the default alignment value is Align middle.
  3. Convert the same Block to a Column block.
  4. Confirm that the default alignment value for is now appropriately set to Top.

Screencast

Screen.Recording.2025-01-24.at.12.17.01.PM.mov

Screenshots

Before After
before after

@yogeshbhutkar yogeshbhutkar mentioned this pull request Jan 24, 2025
Open
@yogeshbhutkar yogeshbhutkar marked this pull request as ready for review January 24, 2025 07:11
@github-actions
Copy link

github-actions bot commented Jan 24, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Block editor /packages/block-editor [Feature] Layout Layout block support, its UI controls, and style output. labels Jan 25, 2025
@yogeshbhutkar yogeshbhutkar force-pushed the fix-68864/incorrect-default-value branch from 7e9a4f1 to 9911076 Compare January 28, 2025 04:55
@yogeshbhutkar yogeshbhutkar force-pushed the fix-68864/incorrect-default-value branch 2 times, most recently from 38ce8a9 to 5e01c20 Compare May 26, 2025 05:09
@yogeshbhutkar
Copy link
Contributor Author

yogeshbhutkar commented May 26, 2025

I noticed that this bug is still unresolved on trunk. Here's more context on the proposed changes to help this PR move forward.

Within flex.js, we're trying to pass the value of verticalAlignment to BlockVerticalAlignmentControl.

gutenberg/packages/block-editor/src/layouts/flex.js

Line 219 in a5d2887

value={ verticalAlignment }

The value of verticalAlignment is determined here:

gutenberg/packages/block-editor/src/layouts/flex.js

Lines 202 to 205 in a5d2887

const defaultVerticalAlignment =
orientation === 'horizontal'
? verticalAlignmentMap.center
: verticalAlignmentMap.top;

When the orientation is vertical (i.e., Stack), verticalAlignmentMap.top becomes flex-start.

gutenberg/packages/block-editor/src/layouts/flex.js

Line 52 in a5d2887

top: 'flex-start',

However, BlockVerticalAlignmentUI does not expect to receive flex-start, there's no corresponding value for flex-start, instead it expects the value top.

gutenberg/packages/block-editor/src/components/block-vertical-alignment-control/ui.js

Line 15 in a5d2887

top: {

This can be seen in the UI as well, when a row block is transformed to stack, the default value is not shown as selected in the UI.

Actual Expected
bug expected

We cannot update verticalAlignmentMap.top as it's also being used elsewhere to add the corresponding justify-content rule, therefore, this PR creates a new constant for the default values.

Hi @t-hamano,
If you have some time, could you please review this PR?

t-hamano
t-hamano reviewed May 29, 2025
View reviewed changes
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

@yogeshbhutkar Thanks for the PR!

To understand the problem exactly, let me confirm one thing.

This problem does not occur in horizontal layout, i.e. Row block, and "Align middle" is selected as the default value correctly. The reason for this is that the default vertical alignment in the horizontal layout is "center", and the key name and value in the verticalAlignmentMap object were the same, so fortunately, no problems occurred.

I my understanding correct?

t-hamano
t-hamano reviewed May 29, 2025
View reviewed changes
@yogeshbhutkar yogeshbhutkar force-pushed the fix-68864/incorrect-default-value branch from 50a74f4 to 1eeed49 Compare May 30, 2025 04:06
@yogeshbhutkar
Copy link
Contributor Author

yogeshbhutkar commented May 30, 2025

This problem does not occur in horizontal layout, i.e. Row block, and "Align middle" is selected as the default value correctly. The reason for this is that the default vertical alignment in the horizontal layout is "center", and the key name and value in the verticalAlignmentMap object were the same, so fortunately, no problems occurred.

I my understanding correct?

Yes, exactly.

t-hamano
t-hamano approved these changes May 30, 2025
View reviewed changes
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

The changes make sense to me 👍

@t-hamano t-hamano merged commit c6b0f2b into WordPress:trunk May 30, 2025
60 checks passed
@github-actions github-actions bot added this to the Gutenberg 21.0 milestone May 30, 2025
chriszarate pushed a commit to chriszarate/gutenberg that referenced this pull request Jul 1, 2025
@yogeshbhutkar @t-hamano @chriszarate
Flex Layout: Fix incorrect default alignment values for `Vertical Ali…
ac432ea 
…gnment Control` component (WordPress#68866)

* Flex Layout: Fix incorrect default alignment values for `Vertical Alignment Control` component

* chore: fix casing of `defaultAlignments`

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@t-hamano t-hamano t-hamano approved these changes

@tellthemachines tellthemachines Awaiting requested review from tellthemachines tellthemachines is a code owner

Assignees

No one assigned

Labels

[Feature] Layout Layout block support, its UI controls, and style output. [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

Gutenberg 21.0

Development

Successfully merging this pull request may close these issues.

Flex Layout: Incorrect default alignment value used in Vertical Alignment Control Component

2 participants

@yogeshbhutkar @t-hamano