Fixed vertical alignment in flex layouts#335
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses frontend vertical alignment issues for menu icons inside flex containers by augmenting the generated icon styles with align-self (flex-friendly) handling, and by narrowing/normalizing the available vertical alignment values.
Changes:
- Removed unsupported/legacy vertical-align options from the settings UI.
- Added backward-compatibility normalization for stored
vertical_alignvalues. - Updated frontend inline-style generation to add
align-selfderived fromvertical_align, plus a helper to map values.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
includes/settings.php |
Removes several vertical-align choices from the settings field. |
includes/meta.php |
Normalizes legacy vertical_align values to supported ones for backward compatibility. |
includes/front.php |
Adds align-self output and mapping logic during icon inline-style generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Backward-compatibility: values removed in favour of align-self support. | ||
| $supported_vertical_align = array( 'top', 'middle', 'bottom', 'baseline' ); | ||
| if ( isset( $value['vertical_align'] ) && | ||
| ! in_array( $value['vertical_align'], $supported_vertical_align, true ) | ||
| ) { | ||
| $value['vertical_align'] = 'middle'; | ||
| } |
There was a problem hiding this comment.
This backward-compatibility block overwrites any previously-saved vertical_align values outside the new supported list (e.g. super, sub, text-top, text-bottom) to middle. Those legacy values are still valid CSS for non-flex layouts, so this migration can cause a visible change for existing sites/themes that are not using flex menus. Consider preserving the legacy vertical_align value for the vertical-align: CSS output, while only mapping/fallbacking align-self to a supported value (or providing a more explicit migration strategy).
| // Backward-compatibility: values removed in favour of align-self support. | |
| $supported_vertical_align = array( 'top', 'middle', 'bottom', 'baseline' ); | |
| if ( isset( $value['vertical_align'] ) && | |
| ! in_array( $value['vertical_align'], $supported_vertical_align, true ) | |
| ) { | |
| $value['vertical_align'] = 'middle'; | |
| } | |
| // Preserve legacy vertical-align values for backward compatibility. |
| 'vertical_align' => array( | ||
| 'id' => 'vertical_align', | ||
| 'type' => 'select', | ||
| 'label' => __( 'Vertical Align', 'menu-icons' ), | ||
| 'default' => 'middle', |
There was a problem hiding this comment.
The vertical_align settings choices have been reduced to 4 values. Combined with the migration in Menu_Icons_Meta::get() that forces legacy values to middle, this is a backwards-incompatible change for users who previously selected super/sub/text-top/text-bottom on non-flex menus (those CSS values are still valid outside flex). Consider keeping the legacy options (perhaps marked as deprecated/“legacy”) or ensuring the migration preserves prior behavior where possible.
|
🎉 This PR is included in version 0.13.23 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Handled vertical alignment for elements inside flex containers by using
align-selfinstead ofvertical-align, which is not supported in flex layouts.Check before Pull Request is ready:
Closes #271