-
Notifications
You must be signed in to change notification settings - Fork 810
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
(WIP) Forms: Update form field support for container block layouts #41805
base: trunk
Are you sure you want to change the base?
(WIP) Forms: Update form field support for container block layouts #41805
Conversation
…, this was wrappers around blocks added by HOCs are still supported
…ld classname handles this and it should only apply when directly within a form, not within another parent that provides a different layout
…n constrained layout
@@ -26,6 +26,7 @@ const FieldDefaults = { | |||
supports: { | |||
reusable: false, | |||
html: false, | |||
spacing: {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be needed to show the sizing controls in the Styles tab. 🤔
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
} | ||
isShownByDefault | ||
> | ||
<JetpackFieldWidth setAttributes={ setAttributes } width={ width } /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved the width field to the styles tab, so that dimension controls (whether core's or jetpack's) are always in a consistent place.
I guess this might need some discussion, and particularly given all the other style controls are still in the settings tab.
Significance: patch | ||
Type: fixed | ||
|
||
Forms: fix column block width when used in the form block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to update this
/** | ||
* Build the CSS for the child layout. | ||
* This replicates part of the `wp_render_layout_support_flag` function from WordPress core. | ||
* That function doesn't work for form fields due to the way they're rendered as shortcodes, | ||
* while the core function will only generate and apply classnames for layout to html. | ||
* | ||
* @param array $layout - the block's `style.layout` attribute. | ||
* | ||
* @return string CSS for the child layout. | ||
*/ | ||
private static function build_child_layout_css( $layout ) { | ||
$css = ''; | ||
|
||
if ( ! $layout ) { | ||
return $css; | ||
} | ||
|
||
$self_stretch = isset( $layout['selfStretch'] ) ? $layout['selfStretch'] : null; | ||
|
||
if ( 'fixed' === $self_stretch && isset( $layout['flexSize'] ) ) { | ||
$css .= 'flex-basis: ' . $layout['flexSize'] . ';'; | ||
$css .= 'box-sizing: border-box;'; | ||
} elseif ( 'fill' === $self_stretch ) { | ||
$css .= 'flex-grow: 1;'; | ||
} | ||
|
||
$column_start = isset( $layout['columnStart'] ) ? $layout['columnStart'] : null; | ||
$column_span = isset( $layout['columnSpan'] ) ? $layout['columnSpan'] : null; | ||
if ( $column_start && $column_span ) { | ||
$css .= "grid-column: $column_start / span $column_span;"; | ||
} elseif ( $column_start ) { | ||
$css .= "grid-column: $column_start;"; | ||
} elseif ( $column_span ) { | ||
$css .= "grid-column: span $column_span;"; | ||
} | ||
|
||
$row_start = isset( $layout['rowStart'] ) ? $layout['rowStart'] : null; | ||
$row_span = isset( $layout['rowSpan'] ) ? $layout['rowSpan'] : null; | ||
if ( $row_start && $row_span ) { | ||
$css .= "grid-row: $row_start / span $row_span;"; | ||
} elseif ( $row_start ) { | ||
$css .= "grid-row: $row_start;"; | ||
} elseif ( $row_span ) { | ||
$css .= "grid-row: span $row_span;"; | ||
} | ||
|
||
return $css; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit unfortunate this is needed, but the positive of doing it this way vs. other things I've tried is that it's very easy to understand and super easy to fix any bugs. It doesn't rely on any tricks and isn't much code.
The other options were all too brittle.
@@ -89,7 +89,6 @@ public function __construct( $attributes, $content = null, $form = null ) { | |||
'requiredtext' => null, | |||
'options' => array(), | |||
'id' => null, | |||
'style' => null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I accidentally deleted this. I'm not sure if it's used?
I tested this and it worked really well, it makes it a lot easier to create the complex form layouts in the editor now. Thank you for tackling this! |
Fixes #41428
Remaining issues:
Description
Makes blocks like Row, Stack, Grid, Group, Columns work within the Form block. Users can add field blocks within them and they should adopt the correct layout.
This allows for achieving layouts that weren't possible before (I guess without custom CSS?):
Two fields and a button in a row and a checkbox underneath. The fields are set to 'Grow', while the button doesn't:
Interesting grids
And probably more
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions: