-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Allow post template responsive grid styles to be unset in the UI #76714
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,12 @@ | |
| * External dependencies | ||
| */ | ||
| import clsx from 'clsx'; | ||
| import fastDeepEqual from 'fast-deep-equal/es6/index.js'; | ||
|
|
||
| /** | ||
| * 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 { | ||
|
|
@@ -113,10 +114,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 ) => { | ||
|
|
@@ -281,9 +284,42 @@ 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 = ! fastDeepEqual( | ||
| previousLayoutRef.current, | ||
| layout | ||
| ); | ||
|
|
||
| if ( | ||
| hasLayoutChanged && | ||
| layoutType === 'grid' && | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ] ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit weird that Oh well |
||
|
|
||
| if ( ! posts ) { | ||
| return ( | ||
| <p { ...blockProps }> | ||
|
|
@@ -315,6 +351,7 @@ export default function PostTemplateEdit( { | |
| setDisplayLayout( { | ||
| type: 'grid', | ||
| columnCount, | ||
| minimumColumnWidth: '12rem', | ||
| } ), | ||
| isActive: layoutType === 'grid', | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: reinstate spaces
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
| } | ||
| } | ||
|
|
@@ -58,3 +58,4 @@ | |
| margin-inline-start: auto; | ||
| margin-inline-end: auto; | ||
| } | ||
|
|
||
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.
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
hasOptionalResponsiveGridpurely 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
hasCustomizedGridLayoutto described the state?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.
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.
hasUpdatedResponsiveGridmay be better? I doubt we'll use this attribute in any other block because the problem is specific to post template.