From 297f80b646d416ec0462d2a27ca94aa5c76857a3 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 10 Jul 2024 14:55:16 +1000 Subject: [PATCH 1/4] useGridLayoutSync: Keep layout in sync regardless of whether Grid is selected --- packages/block-editor/src/hooks/grid-visualizer.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/hooks/grid-visualizer.js b/packages/block-editor/src/hooks/grid-visualizer.js index 42b45952d45d15..ffad5edd9842c4 100644 --- a/packages/block-editor/src/hooks/grid-visualizer.js +++ b/packages/block-editor/src/hooks/grid-visualizer.js @@ -26,14 +26,12 @@ function GridTools( { clientId, layout } ) { }; } ); - if ( ! isSelected && ! isDragging ) { - return null; - } - return ( <> - + { ( isSelected || isDragging ) && ( + + ) } ); } From 9c5e81117a59d463f5064bbb69e313dfae06eebe Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Thu, 11 Jul 2024 11:19:43 +1000 Subject: [PATCH 2/4] Don't use bulk version of updateBlockAttributes() for now. It seems broken --- .../src/components/grid/use-grid-layout-sync.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/grid/use-grid-layout-sync.js b/packages/block-editor/src/components/grid/use-grid-layout-sync.js index a5d6aa4d3fbdd7..2644068cdc01fe 100644 --- a/packages/block-editor/src/components/grid/use-grid-layout-sync.js +++ b/packages/block-editor/src/components/grid/use-grid-layout-sync.js @@ -133,13 +133,12 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { } } - if ( Object.keys( updates ).length ) { + // TODO: We should be able to replace this with a single call to updateBlockAttributes(), + // but there seems to be a bug where getBlocks() selectors don't always update when + // updating attributes this way. + for ( const [ clientId, attributes ] of Object.entries( updates ) ) { __unstableMarkNextChangeAsNotPersistent(); - updateBlockAttributes( - Object.keys( updates ), - updates, - /* uniqueByBlock: */ true - ); + updateBlockAttributes( clientId, attributes ); } }, [ // Actual deps to sync: From 58a546b179fdfb1b5823c014b370357f447bdea3 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Thu, 11 Jul 2024 16:22:42 +1000 Subject: [PATCH 3/4] Revert "Don't use bulk version of updateBlockAttributes() for now. It seems broken" This reverts commit 9c5e81117a59d463f5064bbb69e313dfae06eebe. --- .../src/components/grid/use-grid-layout-sync.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/grid/use-grid-layout-sync.js b/packages/block-editor/src/components/grid/use-grid-layout-sync.js index 2644068cdc01fe..a5d6aa4d3fbdd7 100644 --- a/packages/block-editor/src/components/grid/use-grid-layout-sync.js +++ b/packages/block-editor/src/components/grid/use-grid-layout-sync.js @@ -133,12 +133,13 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { } } - // TODO: We should be able to replace this with a single call to updateBlockAttributes(), - // but there seems to be a bug where getBlocks() selectors don't always update when - // updating attributes this way. - for ( const [ clientId, attributes ] of Object.entries( updates ) ) { + if ( Object.keys( updates ).length ) { __unstableMarkNextChangeAsNotPersistent(); - updateBlockAttributes( clientId, attributes ); + updateBlockAttributes( + Object.keys( updates ), + updates, + /* uniqueByBlock: */ true + ); } }, [ // Actual deps to sync: From 1adc9216dcc9023db468db2f356270a701c08aeb Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Thu, 11 Jul 2024 16:39:39 +1000 Subject: [PATCH 4/4] useGridLayoutSync: Update placement logic to accomodate slash inserter and block splitting --- .../components/grid/use-grid-layout-sync.js | 93 +++++++++++++------ 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/packages/block-editor/src/components/grid/use-grid-layout-sync.js b/packages/block-editor/src/components/grid/use-grid-layout-sync.js index a5d6aa4d3fbdd7..ed368714d63d34 100644 --- a/packages/block-editor/src/components/grid/use-grid-layout-sync.js +++ b/packages/block-editor/src/components/grid/use-grid-layout-sync.js @@ -2,7 +2,8 @@ * WordPress dependencies */ import { useDispatch, useSelect } from '@wordpress/data'; -import { useEffect } from '@wordpress/element'; +import { useEffect, useMemo } from '@wordpress/element'; +import { usePrevious } from '@wordpress/compose'; /** * Internal dependencies @@ -11,13 +12,15 @@ import { store as blockEditorStore } from '../../store'; import { GridRect } from './utils'; export function useGridLayoutSync( { clientId: gridClientId } ) { - const { gridLayout, blockOrder } = useSelect( + const { gridLayout, blockOrder, selectedBlockLayout } = useSelect( ( select ) => { const { getBlockAttributes, getBlockOrder } = select( blockEditorStore ); + const selectedBlock = select( blockEditorStore ).getSelectedBlock(); return { gridLayout: getBlockAttributes( gridClientId ).layout ?? {}, blockOrder: getBlockOrder( gridClientId ), + selectedBlockLayout: selectedBlock?.attributes.style?.layout, }; }, [ gridClientId ] @@ -27,27 +30,32 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { const { updateBlockAttributes, __unstableMarkNextChangeAsNotPersistent } = useDispatch( blockEditorStore ); + const selectedBlockRect = useMemo( + () => + selectedBlockLayout ? new GridRect( selectedBlockLayout ) : null, + [ selectedBlockLayout ] + ); + + const previouslySelectedBlockRect = usePrevious( selectedBlockRect ); + useEffect( () => { const updates = {}; - const { columnCount, rowCount, isManualPlacement } = gridLayout; - - if ( isManualPlacement ) { - const rects = []; + if ( gridLayout.isManualPlacement ) { + const occupiedRects = []; // Respect the position of blocks that already have a columnStart and rowStart value. for ( const clientId of blockOrder ) { - const attributes = getBlockAttributes( clientId ); const { columnStart, rowStart, columnSpan = 1, rowSpan = 1, - } = attributes.style?.layout || {}; + } = getBlockAttributes( clientId ).style?.layout ?? {}; if ( ! columnStart || ! rowStart ) { continue; } - rects.push( + occupiedRects.push( new GridRect( { columnStart, rowStart, @@ -65,17 +73,19 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { rowStart, columnSpan = 1, rowSpan = 1, - } = attributes.style?.layout || {}; + } = attributes.style?.layout ?? {}; if ( columnStart && rowStart ) { continue; } - const [ newColumnStart, newRowStart ] = getFirstEmptyCell( - rects, - columnCount, + const [ newColumnStart, newRowStart ] = placeBlock( + occupiedRects, + gridLayout.columnCount, columnSpan, - rowSpan + rowSpan, + previouslySelectedBlockRect?.columnEnd, + previouslySelectedBlockRect?.rowEnd ); - rects.push( + occupiedRects.push( new GridRect( { columnStart: newColumnStart, rowStart: newRowStart, @@ -96,8 +106,13 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { } // Ensure there's enough rows to fit all blocks. - const bottomMostRow = Math.max( ...rects.map( ( r ) => r.rowEnd ) ); - if ( ! rowCount || rowCount < bottomMostRow ) { + const bottomMostRow = Math.max( + ...occupiedRects.map( ( r ) => r.rowEnd ) + ); + if ( + ! gridLayout.rowCount || + gridLayout.rowCount < bottomMostRow + ) { updates[ gridClientId ] = { layout: { ...gridLayout, @@ -110,7 +125,7 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { for ( const clientId of blockOrder ) { const attributes = getBlockAttributes( clientId ); const { columnStart, rowStart, ...layout } = - attributes.style?.layout || {}; + attributes.style?.layout ?? {}; // Only update attributes if columnStart or rowStart are set. if ( columnStart || rowStart ) { updates[ clientId ] = { @@ -123,7 +138,7 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { } // Remove row styles in auto mode - if ( rowCount ) { + if ( gridLayout.rowCount ) { updates[ gridClientId ] = { layout: { ...gridLayout, @@ -146,23 +161,47 @@ export function useGridLayoutSync( { clientId: gridClientId } ) { gridClientId, gridLayout, blockOrder, - // Needed for linter: + previouslySelectedBlockRect, + // These won't change, but the linter thinks they might: __unstableMarkNextChangeAsNotPersistent, getBlockAttributes, updateBlockAttributes, ] ); } -function getFirstEmptyCell( rects, columnCount, columnSpan = 1, rowSpan = 1 ) { - for ( let row = 1; ; row++ ) { - for ( let column = 1; column <= columnCount; column++ ) { - const rect = new GridRect( { +/** + * @param {GridRect[]} occupiedRects + * @param {number} gridColumnCount + * @param {number} blockColumnSpan + * @param {number} blockRowSpan + * @param {number?} startColumn + * @param {number?} startRow + */ +function placeBlock( + occupiedRects, + gridColumnCount, + blockColumnSpan, + blockRowSpan, + startColumn = 1, + startRow = 1 +) { + for ( let row = startRow; ; row++ ) { + for ( + let column = row === startRow ? startColumn : 1; + column <= gridColumnCount; + column++ + ) { + const candidateRect = new GridRect( { columnStart: column, rowStart: row, - columnSpan, - rowSpan, + columnSpan: blockColumnSpan, + rowSpan: blockRowSpan, } ); - if ( ! rects.some( ( r ) => r.intersectsRect( rect ) ) ) { + if ( + ! occupiedRects.some( ( r ) => + r.intersectsRect( candidateRect ) + ) + ) { return [ column, row ]; } }