Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/reference-guides/core-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,7 @@ Contains the block elements used to render a post, like the title, date, feature
- **Category:** theme
- **Ancestor:** core/query
- **Supports:** align (full, wide), anchor, color (background, gradients, link, text), interactivity (clientNavigation), layout, spacing (blockGap, margin, padding), typography (fontSize, lineHeight), ~~html~~, ~~reusable~~
- **Attributes:** hasOptionalResponsiveGrid

## Post Terms

Expand Down
5 changes: 5 additions & 0 deletions packages/block-library/src/post-template/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
"enhancedPagination",
"postType"
],
"attributes": {
"hasOptionalResponsiveGrid": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just so I understand, when true, this attribute means something like "this block should not use the old behavior" (adds :not(.has-optional-responsive-grid))?

Is hasOptionalResponsiveGrid purely for internal/backwards compatibility, or will it relate to a user-facing toggle later?

If for the latter, I think the naming makes sense since it sounds like user facing controls. 👍🏻

If not, maybe hasCustomizedGridLayout to described the state?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Purely internal but it is linked to an added classname so best if they both match. Not sure where we should classify classnames as user facing though 😄

Ideally the name describes the purpose of the attribute/class but I'm pretty terrible at naming lol. hasUpdatedResponsiveGrid may be better? I doubt we'll use this attribute in any other block because the problem is specific to post template.

"type": "boolean"
}
},
"supports": {
"anchor": true,
"reusable": false,
Expand Down
39 changes: 37 additions & 2 deletions packages/block-library/src/post-template/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import clsx from 'clsx';
/**
* WordPress dependencies
*/
import { memo, useMemo, useState } from '@wordpress/element';
import { memo, useEffect, useMemo, useRef, useState } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { __, _x } from '@wordpress/i18n';
import {
Expand Down Expand Up @@ -113,10 +113,12 @@ export default function PostTemplateEdit( {
templateSlug,
previewPostType,
},
attributes: { layout },
attributes: { layout, hasOptionalResponsiveGrid },
__unstableLayoutClassNames,
} ) {
const { type: layoutType, columnCount = 3 } = layout || {};
const previousLayoutRef = useRef( layout );
const isFirstRenderRef = useRef( true );
const [ activeBlockContextId, setActiveBlockContextId ] = useState();
const { posts, blocks } = useSelect(
( select ) => {
Expand Down Expand Up @@ -281,9 +283,41 @@ export default function PostTemplateEdit( {
className: clsx( __unstableLayoutClassNames, {
[ `columns-${ columnCount }` ]:
layoutType === 'grid' && columnCount, // Ensure column count is flagged via classname for backwards compatibility.
'has-optional-responsive-grid':
layoutType === 'grid' && hasOptionalResponsiveGrid,
} ),
} );

useEffect( () => {
if ( isFirstRenderRef.current ) {
isFirstRenderRef.current = false;
previousLayoutRef.current = layout;

return;
}

const hasLayoutChanged =
JSON.stringify( previousLayoutRef.current ) !==
JSON.stringify( layout );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know if it matters, but this will return false if the order of properties ever changes.

fastDeepEqual is in the package I think. It compares values.

fastDeepEqual( previous, current )


if (
hasLayoutChanged &&
layoutType === 'grid' &&
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So if I change my columns count from 3 to 4 or interact with the grid in any way, I'm flipped from the legacy behaviour, right?

layout?.columnCount &&
! hasOptionalResponsiveGrid
) {
setAttributes( { hasOptionalResponsiveGrid: true } );
} else if (
hasLayoutChanged &&
( layoutType !== 'grid' || ! layout?.columnCount ) &&
hasOptionalResponsiveGrid
) {
setAttributes( { hasOptionalResponsiveGrid: undefined } );
}

previousLayoutRef.current = layout;
}, [ hasOptionalResponsiveGrid, layout, layoutType, setAttributes ] );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A bit weird that exhaustive-deps required layoutType when it's a prop of layout anyway.

Oh well


if ( ! posts ) {
return (
<p { ...blockProps }>
Expand Down Expand Up @@ -315,6 +349,7 @@ export default function PostTemplateEdit( {
setDisplayLayout( {
type: 'grid',
columnCount,
minimumColumnWidth: '12rem',
} ),
isActive: layoutType === 'grid',
},
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/post-template/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ function render_block_core_post_template( $attributes, $content, $block ) {
// Ensure backwards compatibility by flagging the number of columns via classname when using grid layout.
if ( isset( $attributes['layout']['type'] ) && 'grid' === $attributes['layout']['type'] && ! empty( $attributes['layout']['columnCount'] ) ) {
$classnames .= ' ' . sanitize_title( 'columns-' . $attributes['layout']['columnCount'] );

if ( ! empty( $attributes['hasOptionalResponsiveGrid'] ) ) {
$classnames .= ' has-optional-responsive-grid';
}
}

$wrapper_attributes = get_block_wrapper_attributes( array( 'class' => trim( $classnames ) ) );
Expand Down
7 changes: 4 additions & 3 deletions packages/block-library/src/post-template/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@
}
}

@media ( max-width: $break-small ) {
// Temporary specificity bump until "wp-container" layout specificity is revisited.
.wp-block-post-template-is-layout-grid.wp-block-post-template-is-layout-grid.wp-block-post-template-is-layout-grid.wp-block-post-template-is-layout-grid {
// Legacy fallback for older grid layouts before responsive behaviour was implemented.
@media (max-width: $break-small) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: reinstate spaces @media ( max-width: $break-small )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't use spaces in scss files though. If there's anywhere we do it's an accident.

.wp-block-post-template-is-layout-grid[class*="columns-"]:not(.has-optional-responsive-grid) {
grid-template-columns: 1fr;
}
}
Expand All @@ -58,3 +58,4 @@
margin-inline-start: auto;
margin-inline-end: auto;
}

Loading