From eaec4251199389638be65b96a7c924d1c222b075 Mon Sep 17 00:00:00 2001 From: Bryam Rodriguez Date: Thu, 14 Nov 2024 19:24:50 -0600 Subject: [PATCH 1/5] Implement new _getModulePositioningConfig --- packages/gestalt/src/Masonry.tsx | 46 +++++++++++----- packages/gestalt/src/Masonry/defaultLayout.ts | 22 ++++++-- .../src/Masonry/fullWidthLayout.test.ts | 5 ++ .../gestalt/src/Masonry/fullWidthLayout.ts | 30 +++++++---- .../gestalt/src/Masonry/getLayoutAlgorithm.ts | 11 ++-- .../src/Masonry/multiColumnLayout.test.ts | 12 +++-- .../gestalt/src/Masonry/multiColumnLayout.ts | 37 +++++++++---- packages/gestalt/src/MasonryV2.tsx | 54 ++++++++++++++----- 8 files changed, 155 insertions(+), 62 deletions(-) diff --git a/packages/gestalt/src/Masonry.tsx b/packages/gestalt/src/Masonry.tsx index 4a58881ddc..9e3519664b 100644 --- a/packages/gestalt/src/Masonry.tsx +++ b/packages/gestalt/src/Masonry.tsx @@ -4,10 +4,16 @@ import FetchItems from './FetchItems'; import styles from './Masonry.css'; import { Cache } from './Masonry/Cache'; import recalcHeights from './Masonry/dynamicHeightsUtils'; +import getColumnCount from './Masonry/getColumnCount'; import getLayoutAlgorithm from './Masonry/getLayoutAlgorithm'; import ItemResizeObserverWrapper from './Masonry/ItemResizeObserverWrapper'; import MeasurementStore from './Masonry/MeasurementStore'; -import { ColumnSpanConfig, MULTI_COL_ITEMS_MEASURE_BATCH_SIZE } from './Masonry/multiColumnLayout'; +import { + calculateActualColumnSpan, + ColumnSpanConfig, + ModulePositioningConfig, + MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, +} from './Masonry/multiColumnLayout'; import ScrollContainer from './Masonry/ScrollContainer'; import { getElementHeight, getRelativeScrollTop, getScrollPos } from './Masonry/scrollUtils'; import { Align, Layout, LoadingStateItem, Position } from './Masonry/types'; @@ -148,11 +154,13 @@ type Props = { */ _dynamicHeights?: boolean; /** - * Experimental prop to enable early bailout when positioning multicolumn modules + * Experimental prop to enable dynamic batch sizing and early bailout when positioning a module + * - Early bailout: How much whitespace is "good enough" + * - Dynamic batch sizing: How many items it can use. If this prop isn't used, it uses 5 * * This is an experimental prop and may be removed or changed in the future */ - _earlyBailout?: (columnSpan: number) => number; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; }; type State = { @@ -589,7 +597,7 @@ export default class Masonry extends ReactComponent, State> { _getColumnSpanConfig, _loadingStateItems = [], _renderLoadingStateItems, - _earlyBailout, + _getModulePositioningConfig, } = this.props; const { hasPendingMeasurements, measurementStore, width } = this.state; const { positionStore } = this; @@ -611,7 +619,7 @@ export default class Masonry extends ReactComponent, State> { _logTwoColWhitespace, _loadingStateItems, renderLoadingState, - _earlyBailout, + _getModulePositioningConfig, }); let gridBody; @@ -701,15 +709,29 @@ export default class Masonry extends ReactComponent, State> { // Full layout is possible const itemsToRender = items.filter((item) => item && measurementStore.has(item)); const itemsWithoutPositions = items.filter((item) => item && !positionStore.has(item)); - const hasMultiColumnItems = + const nextMultiColumnItem = _getColumnSpanConfig && - itemsWithoutPositions.some((item) => _getColumnSpanConfig(item) !== 1); + itemsWithoutPositions.find((item) => _getColumnSpanConfig(item) !== 1); - // If there are 2-col items, we need to measure more items to ensure we have enough possible layouts to find a suitable one - // we need the batch size (number of one column items for the graph) + 1 (two column item) - const itemsToMeasureCount = hasMultiColumnItems - ? MULTI_COL_ITEMS_MEASURE_BATCH_SIZE + 1 - : minCols; + let batchSize; + if (nextMultiColumnItem) { + const gridSize = getColumnCount({ gutter, columnWidth, width, minCols, layout }); + + const moduleSize = calculateActualColumnSpan({ + columnCount: gridSize, + item: nextMultiColumnItem, + _getColumnSpanConfig, + }); + + const { itemsBatchSize } = _getModulePositioningConfig?.(gridSize, moduleSize) || { + itemsBatchSize: MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, + }; + batchSize = itemsBatchSize; + } + + // If there are multicolumn items, we need to measure more items to ensure we have enough possible layouts to find a suitable one + // we need the batch size (number of one column items for the graph) + 1 (multicolumn item) + const itemsToMeasureCount = batchSize ? batchSize + 1 : minCols; const itemsToMeasure = items .filter((item) => item && !measurementStore.has(item)) .slice(0, itemsToMeasureCount); diff --git a/packages/gestalt/src/Masonry/defaultLayout.ts b/packages/gestalt/src/Masonry/defaultLayout.ts index 152c65f162..fc085bfc1e 100644 --- a/packages/gestalt/src/Masonry/defaultLayout.ts +++ b/packages/gestalt/src/Masonry/defaultLayout.ts @@ -1,8 +1,12 @@ import { Cache } from './Cache'; +import getColumnCount, { + DEFAULT_LAYOUT_DEFAULT_COLUMN_WIDTH, + DEFAULT_LAYOUT_DEFAULT_GUTTER, +} from './getColumnCount'; import { getHeightAndGutter, offscreen } from './layoutHelpers'; import { isLoadingStateItem, isLoadingStateItems } from './loadingStateUtils'; import mindex from './mindex'; -import multiColumnLayout, { ColumnSpanConfig } from './multiColumnLayout'; +import multiColumnLayout, { ColumnSpanConfig, ModulePositioningConfig } from './multiColumnLayout'; import { Align, Layout, LoadingStateItem, Position } from './types'; const calculateCenterOffset = ({ @@ -38,14 +42,15 @@ const calculateCenterOffset = ({ const defaultLayout = ({ align, - columnWidth = 236, - gutter = 14, + columnWidth = DEFAULT_LAYOUT_DEFAULT_COLUMN_WIDTH, + gutter = DEFAULT_LAYOUT_DEFAULT_GUTTER, layout, minCols = 2, rawItemCount, width, measurementCache, _getColumnSpanConfig, + _getModulePositioningConfig, renderLoadingState, ...otherProps }: { @@ -59,7 +64,7 @@ const defaultLayout = positionCache: Cache; measurementCache: Cache; _getColumnSpanConfig?: (item: T) => ColumnSpanConfig; - earlyBailout?: (columnSpan: number) => number; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; logWhitespace?: ( additionalWhitespace: ReadonlyArray, numberOfIterations: number, @@ -73,7 +78,13 @@ const defaultLayout = } const columnWidthAndGutter = columnWidth + gutter; - const columnCount = Math.max(Math.floor((width + gutter) / columnWidthAndGutter), minCols); + const columnCount = getColumnCount({ + gutter, + columnWidth, + width, + minCols, + layout, + }); // the total height of each column const heights = new Array(columnCount).fill(0); @@ -96,6 +107,7 @@ const defaultLayout = gutter, measurementCache, _getColumnSpanConfig, + _getModulePositioningConfig, ...otherProps, }) : items.map((item) => { diff --git a/packages/gestalt/src/Masonry/fullWidthLayout.test.ts b/packages/gestalt/src/Masonry/fullWidthLayout.test.ts index e863ef609f..efe48666c2 100644 --- a/packages/gestalt/src/Masonry/fullWidthLayout.test.ts +++ b/packages/gestalt/src/Masonry/fullWidthLayout.test.ts @@ -31,6 +31,7 @@ describe.each([undefined, getColumnSpanConfig])( measurementCache: measurementStore, positionCache, gutter: 10, + layout: 'flexible', idealColumnWidth: 240, minCols: 2, width: 1000, @@ -65,6 +66,7 @@ describe.each([undefined, getColumnSpanConfig])( measurementCache: measurementStore, positionCache, gutter: 10, + layout: 'flexible', idealColumnWidth: 240, minCols: 2, width: 1000, @@ -98,6 +100,7 @@ describe.each([undefined, getColumnSpanConfig])( measurementCache: measurementStore, positionCache, gutter: 10, + layout: 'flexible', idealColumnWidth: 240, minCols: 2, width: 1000, @@ -134,6 +137,7 @@ describe('loadingStateItems', () => { idealColumnWidth: 240, minCols: 2, width: 1000, + layout: 'flexible', _getColumnSpanConfig: getColumnSpanConfig, renderLoadingState: true, }); @@ -170,6 +174,7 @@ describe('loadingStateItems', () => { idealColumnWidth: 240, minCols: 2, width: 1000, + layout: 'flexible', _getColumnSpanConfig: getColumnSpanConfig, }); diff --git a/packages/gestalt/src/Masonry/fullWidthLayout.ts b/packages/gestalt/src/Masonry/fullWidthLayout.ts index 3f6a0ff11a..c7bb6784a7 100644 --- a/packages/gestalt/src/Masonry/fullWidthLayout.ts +++ b/packages/gestalt/src/Masonry/fullWidthLayout.ts @@ -1,28 +1,35 @@ import { Cache } from './Cache'; +import getColumnCount, { + FULL_WIDTH_DEFAULT_GUTTER, + FULL_WIDTH_LAYOUT_DEFAULT_IDEAL_COLUMN_WIDTH, +} from './getColumnCount'; import { getHeightAndGutter } from './layoutHelpers'; import { isLoadingStateItem, isLoadingStateItems } from './loadingStateUtils'; import mindex from './mindex'; -import multiColumnLayout, { ColumnSpanConfig } from './multiColumnLayout'; -import { LoadingStateItem, Position } from './types'; +import multiColumnLayout, { ColumnSpanConfig, ModulePositioningConfig } from './multiColumnLayout'; +import { Layout, LoadingStateItem, Position } from './types'; const fullWidthLayout = ({ width, - idealColumnWidth = 240, - gutter = 0, + idealColumnWidth = FULL_WIDTH_LAYOUT_DEFAULT_IDEAL_COLUMN_WIDTH, + gutter = FULL_WIDTH_DEFAULT_GUTTER, minCols = 2, + layout, measurementCache, _getColumnSpanConfig, + _getModulePositioningConfig, renderLoadingState, ...otherProps }: { idealColumnWidth?: number; gutter?: number; minCols?: number; + layout: Layout; width?: number | null | undefined; positionCache: Cache; measurementCache: Cache; _getColumnSpanConfig?: (item: T) => ColumnSpanConfig; - earlyBailout?: (columnSpan: number) => number; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; logWhitespace?: ( additionalWhitespace: ReadonlyArray, numberOfIterations: number, @@ -40,11 +47,13 @@ const fullWidthLayout = ({ })); } - // "This is kind of crazy!" - you - // Yes, indeed. The "guessing" here is meant to replicate the pass that the - // original implementation takes with CSS. - const colguess = Math.floor(width / idealColumnWidth); - const columnCount = Math.max(Math.floor((width - colguess * gutter) / idealColumnWidth), minCols); + const columnCount = getColumnCount({ + gutter, + columnWidth: idealColumnWidth, + width, + minCols, + layout, + }); const columnWidth = Math.floor(width / columnCount) - gutter; const columnWidthAndGutter = columnWidth + gutter; const centerOffset = gutter / 2; @@ -60,6 +69,7 @@ const fullWidthLayout = ({ gutter, measurementCache, _getColumnSpanConfig, + _getModulePositioningConfig, ...otherProps, }) : items.reduce>((acc, item) => { diff --git a/packages/gestalt/src/Masonry/getLayoutAlgorithm.ts b/packages/gestalt/src/Masonry/getLayoutAlgorithm.ts index 09440dccf9..4220496559 100644 --- a/packages/gestalt/src/Masonry/getLayoutAlgorithm.ts +++ b/packages/gestalt/src/Masonry/getLayoutAlgorithm.ts @@ -1,7 +1,7 @@ import { Cache } from './Cache'; import defaultLayout from './defaultLayout'; import fullWidthLayout from './fullWidthLayout'; -import { ColumnSpanConfig } from './multiColumnLayout'; +import { ColumnSpanConfig, ModulePositioningConfig } from './multiColumnLayout'; import { Align, Layout, LoadingStateItem, Position } from './types'; import uniformRowLayout from './uniformRowLayout'; @@ -19,7 +19,7 @@ export default function getLayoutAlgorithm({ _logTwoColWhitespace, _loadingStateItems = [], renderLoadingState, - _earlyBailout, + _getModulePositioningConfig, }: { align: Align; columnWidth: number | undefined; @@ -31,6 +31,7 @@ export default function getLayoutAlgorithm({ positionStore: Cache; width: number | null | undefined; _getColumnSpanConfig?: (item: T) => ColumnSpanConfig; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; _logTwoColWhitespace?: ( additionalWhitespace: ReadonlyArray, numberOfIterations: number, @@ -38,11 +39,11 @@ export default function getLayoutAlgorithm({ ) => void; _loadingStateItems?: ReadonlyArray; renderLoadingState?: boolean; - _earlyBailout?: (columnSpan: number) => number; }): (items: ReadonlyArray | ReadonlyArray) => ReadonlyArray { if ((layout === 'flexible' || layout === 'serverRenderedFlexible') && width !== null) { return fullWidthLayout({ gutter, + layout, measurementCache: measurementStore, positionCache: positionStore, minCols, @@ -50,8 +51,8 @@ export default function getLayoutAlgorithm({ width, logWhitespace: _logTwoColWhitespace, _getColumnSpanConfig, + _getModulePositioningConfig, renderLoadingState, - earlyBailout: _earlyBailout, }); } if (layout.startsWith('uniformRow')) { @@ -77,7 +78,7 @@ export default function getLayoutAlgorithm({ rawItemCount: renderLoadingState ? _loadingStateItems.length : items.length, width, _getColumnSpanConfig, + _getModulePositioningConfig, renderLoadingState, - earlyBailout: _earlyBailout, }); } diff --git a/packages/gestalt/src/Masonry/multiColumnLayout.test.ts b/packages/gestalt/src/Masonry/multiColumnLayout.test.ts index 8dd8d83c17..dd7e721a90 100644 --- a/packages/gestalt/src/Masonry/multiColumnLayout.test.ts +++ b/packages/gestalt/src/Masonry/multiColumnLayout.test.ts @@ -334,11 +334,13 @@ describe('multi column layout test cases', () => { const gutter = 5; - const earlyBailout = (columnSpan: number) => { - if (columnSpan <= 3) { - return 2 * gutter; + const getModulePositioningConfig = (_: number, moduleSize: number) => { + let whitespaceThreshold = 3 * gutter; + if (moduleSize <= 3) { + whitespaceThreshold = 2 * gutter; } - return 3 * gutter; + + return { itemsBatchSize: 5, whitespaceThreshold }; }; const layout = (itemsToLayout: readonly Item[]) => @@ -350,7 +352,7 @@ describe('multi column layout test cases', () => { centerOffset: 20, measurementCache: measurementStore, positionCache, - earlyBailout, + _getModulePositioningConfig: getModulePositioningConfig, _getColumnSpanConfig: getColumnSpanConfig, }); diff --git a/packages/gestalt/src/Masonry/multiColumnLayout.ts b/packages/gestalt/src/Masonry/multiColumnLayout.ts index 506e1f6c54..e28e212ce6 100644 --- a/packages/gestalt/src/Masonry/multiColumnLayout.ts +++ b/packages/gestalt/src/Masonry/multiColumnLayout.ts @@ -4,14 +4,19 @@ import { getHeightAndGutter, offscreen } from './layoutHelpers'; import mindex from './mindex'; import { GetGraphPositionsReturn, NodeData, Position } from './types'; -// When there's a multi column item in the most recently fetched batch of items, we need to measure more items to ensure we have enough possible layouts to minimize whitespace above the 2-col item -// This may need to be tweaked to balance the tradeoff of delayed rendering vs having enough possible layouts +// When there's a multicolumn item in the most recently fetched batch of items, we need to measure more items to ensure we have enough possible layouts to minimize whitespace above the multicolumn item +// This number can be dynamimcally set using _getModulePositioningConfig export const MULTI_COL_ITEMS_MEASURE_BATCH_SIZE = 5; type GridSize = 'sm' | 'md' | 'lg' | 'xl'; export type ColumnSpanConfig = number | { [Size in GridSize]: number }; +export type ModulePositioningConfig = { + itemsBatchSize: number; // Maximum number of items used to position a module + whitespaceThreshold: number; // "Good enough" whitespace number when positioning a module +}; + // maps the number of columns to a grid breakpoint // sm: 2 columns // md: 3-4 columns @@ -39,7 +44,7 @@ function getPositionsOnly( return positions.map(({ position }) => position); } -function calculateActualColumnSpan(props: { +export function calculateActualColumnSpan(props: { columnCount: number; item: T; _getColumnSpanConfig: (item: T) => ColumnSpanConfig; @@ -102,12 +107,14 @@ function calculateSplitIndex({ emptyColumns, fitsFirstRow, replaceWithOneColItems, + itemsBatchSize, }: { oneColumnItemsLength: number; multiColumnIndex: number; emptyColumns: number; fitsFirstRow: boolean; replaceWithOneColItems: boolean; + itemsBatchSize: number; }): number { // multi column item is on its original position if (fitsFirstRow) { @@ -121,9 +128,9 @@ function calculateSplitIndex({ // If two column module is near the end of the batch // we move the index so it has enough items for the graph - if (multiColumnIndex + MULTI_COL_ITEMS_MEASURE_BATCH_SIZE > oneColumnItemsLength) { + if (multiColumnIndex + itemsBatchSize > oneColumnItemsLength) { return Math.max( - oneColumnItemsLength - MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, + oneColumnItemsLength - itemsBatchSize, // We have to keep at least the items for the empty columns to fill emptyColumns, ); @@ -474,10 +481,10 @@ function getPositionsWithMultiColumnItem({ itemsToPosition, heights, prevPositions, - earlyBailout, columnCount, logWhitespace, _getColumnSpanConfig, + _getModulePositioningConfig, ...commonGetPositionArgs }: { multiColumnItem: T; @@ -487,7 +494,6 @@ function getPositionsWithMultiColumnItem({ item: T; position: Position; }>; - earlyBailout?: (columnSpan: number) => number; logWhitespace?: ( additionalWhitespace: ReadonlyArray, numberOfIterations: number, @@ -501,6 +507,7 @@ function getPositionsWithMultiColumnItem({ measurementCache: Cache; positionCache: Cache; _getColumnSpanConfig: (item: T) => ColumnSpanConfig; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; }): { positions: ReadonlyArray<{ item: T; @@ -535,6 +542,13 @@ function getPositionsWithMultiColumnItem({ // we need to fill those spaces with one col items const replaceWithOneColItems = !fitsFirstRow && multiColumnIndex < emptyColumns; + const { itemsBatchSize, whitespaceThreshold } = _getModulePositioningConfig?.( + columnCount, + multiColumnItemColumnSpan, + ) || { + itemsBatchSize: MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, + }; + // Calculate how many items are on pre array and how many on graphBatch // pre items are positioned before the two column item const splitIndex = calculateSplitIndex({ @@ -543,12 +557,13 @@ function getPositionsWithMultiColumnItem({ emptyColumns, fitsFirstRow, replaceWithOneColItems, + itemsBatchSize, }); const pre = oneColumnItems.slice(0, splitIndex); const graphBatch = fitsFirstRow ? [] - : oneColumnItems.slice(splitIndex, splitIndex + MULTI_COL_ITEMS_MEASURE_BATCH_SIZE); + : oneColumnItems.slice(splitIndex, splitIndex + itemsBatchSize); // Get positions and heights for painted items const { positions: paintedItemPositions, heights: paintedItemHeights } = @@ -631,8 +646,8 @@ const multiColumnLayout = ({ logWhitespace, measurementCache, positionCache, - earlyBailout, _getColumnSpanConfig, + _getModulePositioningConfig, }: { items: ReadonlyArray; gutter?: number; @@ -641,13 +656,13 @@ const multiColumnLayout = ({ centerOffset?: number; positionCache: Cache; measurementCache: Cache; - earlyBailout?: (columnSpan: number) => number; logWhitespace?: ( additionalWhitespace: ReadonlyArray, numberOfIterations: number, columnSpan: number, ) => void; _getColumnSpanConfig: (item: T) => ColumnSpanConfig; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; }): ReadonlyArray => { if (!items.every((item) => measurementCache.has(item))) { return items.map((item) => { @@ -723,10 +738,10 @@ const multiColumnLayout = ({ itemsToPosition, heights: acc.heights, prevPositions: acc.positions, - earlyBailout, logWhitespace, columnCount, _getColumnSpanConfig, + _getModulePositioningConfig, ...commonGetPositionArgs, }), { heights: paintedItemHeights, positions: paintedItemPositions }, diff --git a/packages/gestalt/src/MasonryV2.tsx b/packages/gestalt/src/MasonryV2.tsx index 73ce5a9b54..171bc8b056 100644 --- a/packages/gestalt/src/MasonryV2.tsx +++ b/packages/gestalt/src/MasonryV2.tsx @@ -16,10 +16,16 @@ import debounce from './debounce'; import styles from './Masonry.css'; import { Cache } from './Masonry/Cache'; import recalcHeights from './Masonry/dynamicHeightsUtils'; +import getColumnCount from './Masonry/getColumnCount'; import getLayoutAlgorithm from './Masonry/getLayoutAlgorithm'; import ItemResizeObserverWrapper from './Masonry/ItemResizeObserverWrapper'; import MeasurementStore from './Masonry/MeasurementStore'; -import { ColumnSpanConfig, MULTI_COL_ITEMS_MEASURE_BATCH_SIZE } from './Masonry/multiColumnLayout'; +import { + calculateActualColumnSpan, + ColumnSpanConfig, + ModulePositioningConfig, + MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, +} from './Masonry/multiColumnLayout'; import { getElementHeight, getRelativeScrollTop, getScrollPos } from './Masonry/scrollUtils'; import { Align, Layout, LoadingStateItem, Position } from './Masonry/types'; import throttle from './throttle'; @@ -162,11 +168,13 @@ type Props = { */ _dynamicHeights?: boolean; /** - * Experimental prop to enable early bailout when positioning multicolumn modules + * Experimental prop to enable dynamic batch sizing and early bailout when positioning a module + * - Early bailout: How much whitespace is "good enough" + * - Dynamic batch sizing: How many items it can use. If this prop isn't used, it uses 5 * * This is an experimental prop and may be removed or changed in the future */ - _earlyBailout?: (columnSpan: number) => number; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; }; type MasonryRef = { @@ -376,7 +384,7 @@ function useLayout({ _getColumnSpanConfig, _loadingStateItems = [], _renderLoadingStateItems, - _earlyBailout, + _getModulePositioningConfig, }: { align: Align; columnWidth: number; @@ -398,7 +406,7 @@ function useLayout({ _getColumnSpanConfig?: (item: T) => ColumnSpanConfig; _loadingStateItems?: ReadonlyArray; _renderLoadingStateItems?: Props['_renderLoadingStateItems']; - _earlyBailout?: (columnSpan: number) => number; + _getModulePositioningConfig?: (gridSize: number, moduleSize: number) => ModulePositioningConfig; }): { height: number; hasPendingMeasurements: boolean; @@ -424,15 +432,33 @@ function useLayout({ _logTwoColWhitespace, _loadingStateItems, renderLoadingState, - _earlyBailout, + _getModulePositioningConfig, }); - const hasMultiColumnItems = - _getColumnSpanConfig && - items - .filter((item) => item && !positionStore.has(item)) - .some((item) => _getColumnSpanConfig(item) !== 1); - const itemToMeasureCount = hasMultiColumnItems ? MULTI_COL_ITEMS_MEASURE_BATCH_SIZE + 1 : minCols; + const nextMultiColumnItem = + _getColumnSpanConfig && items.find((item) => _getColumnSpanConfig(item) !== 1); + + let batchSize; + if (nextMultiColumnItem) { + if (width) { + const gridSize = getColumnCount({ gutter, columnWidth, width, minCols, layout }); + + const moduleSize = calculateActualColumnSpan({ + columnCount: gridSize, + item: nextMultiColumnItem, + _getColumnSpanConfig, + }); + + const { itemsBatchSize } = _getModulePositioningConfig?.(gridSize, moduleSize) || { + itemsBatchSize: MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, + }; + batchSize = itemsBatchSize; + } else { + batchSize = MULTI_COL_ITEMS_MEASURE_BATCH_SIZE; + } + } + + const itemToMeasureCount = batchSize ? batchSize + 1 : minCols; const itemMeasurements = items.filter((item) => measurementStore.has(item)); const itemMeasurementsCount = itemMeasurements.length; const hasPendingMeasurements = itemMeasurementsCount < items.length; @@ -701,7 +727,7 @@ function Masonry( _dynamicHeights, _loadingStateItems = [], _renderLoadingStateItems, - _earlyBailout, + _getModulePositioningConfig, }: Props, ref: | { @@ -825,7 +851,7 @@ function Masonry( _getColumnSpanConfig, _loadingStateItems, _renderLoadingStateItems, - _earlyBailout, + _getModulePositioningConfig, }); useFetchOnScroll({ From 048fa6d513be70af037dae94593c6f6b23f9c678 Mon Sep 17 00:00:00 2001 From: Bryam Rodriguez Date: Thu, 14 Nov 2024 19:29:25 -0600 Subject: [PATCH 2/5] Set positioning config in testing --- docs/integration-test-helpers/masonry/MasonryContainer.tsx | 4 ++++ packages/gestalt/src/Masonry/multiColumnLayout.ts | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/integration-test-helpers/masonry/MasonryContainer.tsx b/docs/integration-test-helpers/masonry/MasonryContainer.tsx index cb95623bc5..ca8e88df45 100644 --- a/docs/integration-test-helpers/masonry/MasonryContainer.tsx +++ b/docs/integration-test-helpers/masonry/MasonryContainer.tsx @@ -412,6 +412,10 @@ export default class MasonryContainer extends Component>, const columnSpan = item.columnSpan as number | undefined; return columnSpan ?? 1; }} + _getModulePositioningConfig={() => ({ + itemsBatchSize: 6, + whitespaceThreshold: 32, + })} _logTwoColWhitespace={ logWhitespace ? // eslint-disable-next-line no-console diff --git a/packages/gestalt/src/Masonry/multiColumnLayout.ts b/packages/gestalt/src/Masonry/multiColumnLayout.ts index e28e212ce6..6d733cbc09 100644 --- a/packages/gestalt/src/Masonry/multiColumnLayout.ts +++ b/packages/gestalt/src/Masonry/multiColumnLayout.ts @@ -578,8 +578,6 @@ function getPositionsWithMultiColumnItem({ positionCache.set(item, position); }); - const whitespaceThreshold = earlyBailout?.(multiColumnItemColumnSpan); - // Get a node with the required whitespace const { winningNode, numberOfIterations } = getGraphPositions({ items: graphBatch, From a2a7ce0a01987b47fd89b83811e449994f794256 Mon Sep 17 00:00:00 2001 From: Bryam Rodriguez Date: Thu, 14 Nov 2024 19:45:52 -0600 Subject: [PATCH 3/5] Add getColumnCount to prod dist --- .../gestalt/src/Masonry/getColumnCount.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 packages/gestalt/src/Masonry/getColumnCount.ts diff --git a/packages/gestalt/src/Masonry/getColumnCount.ts b/packages/gestalt/src/Masonry/getColumnCount.ts new file mode 100644 index 0000000000..99b1cd1516 --- /dev/null +++ b/packages/gestalt/src/Masonry/getColumnCount.ts @@ -0,0 +1,39 @@ +import { Layout } from './types'; + +export const FULL_WIDTH_LAYOUT_DEFAULT_IDEAL_COLUMN_WIDTH = 240; +export const FULL_WIDTH_DEFAULT_GUTTER = 0; +export const DEFAULT_LAYOUT_DEFAULT_COLUMN_WIDTH = 236; +export const DEFAULT_LAYOUT_DEFAULT_GUTTER = 14; + +export default function getColumnCount({ + gutter, + columnWidth, + width, + minCols, + layout, +}: { + gutter: number | undefined; + columnWidth: number | undefined; + width: number; + minCols: number; + layout: Layout; +}): number { + if (layout === 'flexible' || (layout === 'serverRenderedFlexible' && width !== null)) { + // "This is kind of crazy!" - you + // Yes, indeed. The "guessing" here is meant to replicate the pass that the + // original implementation takes with CSS. + const idealColumnWidth = + typeof columnWidth === 'undefined' + ? FULL_WIDTH_LAYOUT_DEFAULT_IDEAL_COLUMN_WIDTH + : columnWidth; + const idealGutter = typeof gutter === 'undefined' ? FULL_WIDTH_DEFAULT_GUTTER : gutter; + const colguess = Math.floor(width / idealColumnWidth); + return Math.max(Math.floor((width - colguess * idealGutter) / idealColumnWidth), minCols); + } + + const idealGutter = typeof gutter === 'undefined' ? DEFAULT_LAYOUT_DEFAULT_GUTTER : gutter; + const idealColumnWidth = + typeof columnWidth === 'undefined' ? DEFAULT_LAYOUT_DEFAULT_COLUMN_WIDTH : columnWidth; + const columnWidthAndGutter = idealColumnWidth + idealGutter; + return Math.max(Math.floor((width + idealGutter) / columnWidthAndGutter), minCols); +} From 59f0fad566627f62b1e624e714121c7bc15733d8 Mon Sep 17 00:00:00 2001 From: Bryam Rodriguez Date: Tue, 26 Nov 2024 11:23:27 -0600 Subject: [PATCH 4/5] Add gutter to Masonry and MasonryV2 main components --- packages/gestalt/src/Masonry/defaultLayout.test.ts | 10 ++++++++++ .../gestalt/src/Masonry/dynamicHeightsUtils.test.ts | 12 ++++++------ packages/gestalt/src/Masonry/getColumnCount.ts | 1 - packages/gestalt/src/Masonry/multiColumnLayout.ts | 1 - .../gestalt/src/Masonry/uniformRowLayout.test.ts | 7 +++++++ 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/gestalt/src/Masonry/defaultLayout.test.ts b/packages/gestalt/src/Masonry/defaultLayout.test.ts index 8f6e262e19..b7c6ae05f7 100644 --- a/packages/gestalt/src/Masonry/defaultLayout.test.ts +++ b/packages/gestalt/src/Masonry/defaultLayout.test.ts @@ -1,4 +1,5 @@ import defaultLayout from './defaultLayout'; +import { DEFAULT_LAYOUT_DEFAULT_GUTTER } from './getColumnCount'; import MeasurementStore from './MeasurementStore'; import { Position } from './types'; @@ -27,6 +28,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'start', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -58,6 +60,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'center', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -90,6 +93,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'center', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basicCentered', @@ -122,6 +126,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'end', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -154,6 +159,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'start', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -185,6 +191,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'start', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -266,6 +273,7 @@ describe.each([undefined, getColumnSpanConfig])('default layout tests', (_getCol const layout = defaultLayout({ align: 'end', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -299,6 +307,7 @@ describe('loadingStateItems', () => { const layout = defaultLayout({ align: 'start', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', @@ -335,6 +344,7 @@ describe('loadingStateItems', () => { const layout = defaultLayout({ align: 'start', + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, measurementCache: measurementStore, positionCache, layout: 'basic', diff --git a/packages/gestalt/src/Masonry/dynamicHeightsUtils.test.ts b/packages/gestalt/src/Masonry/dynamicHeightsUtils.test.ts index ea64cdebe9..d298c1d7b4 100644 --- a/packages/gestalt/src/Masonry/dynamicHeightsUtils.test.ts +++ b/packages/gestalt/src/Masonry/dynamicHeightsUtils.test.ts @@ -58,7 +58,7 @@ describe('dynamic heights on masonry', () => { newHeight: items[changedItemIndex].height + heightDelta, positionStore: positionCache, measurementStore, - gutterWidth: gutter, + gutter, }); items.forEach((item, index) => { @@ -114,7 +114,7 @@ describe('dynamic heights on masonry', () => { newHeight: items[changedItemIndex].height - heightDelta, positionStore: positionCache, measurementStore, - gutterWidth: gutter, + gutter, }); items.forEach((item, index) => { @@ -168,7 +168,7 @@ describe('dynamic heights on masonry', () => { newHeight: items[changedItemIndex].height - heightDelta, positionStore: positionCache, measurementStore, - gutterWidth: gutter, + gutter, }); items.forEach((item, index) => { @@ -223,7 +223,7 @@ describe('dynamic heights on masonry', () => { newHeight: firstItemNewHeight, positionStore: positionCache, measurementStore, - gutterWidth: gutter, + gutter, }); const changedItemIndex2 = 1; @@ -234,7 +234,7 @@ describe('dynamic heights on masonry', () => { newHeight: items[changedItemIndex2].height + heightDelta, positionStore: positionCache, measurementStore, - gutterWidth: gutter, + gutter, }); const twoColItemIndex = 2; @@ -326,7 +326,7 @@ describe('dynamic heights on masonry', () => { newHeight: changedItemIndexNewHeight, positionStore: positionCache, measurementStore, - gutterWidth: gutter, + gutter, }); const expectedPos = [ diff --git a/packages/gestalt/src/Masonry/getColumnCount.ts b/packages/gestalt/src/Masonry/getColumnCount.ts index dede908866..9b0b9746f1 100644 --- a/packages/gestalt/src/Masonry/getColumnCount.ts +++ b/packages/gestalt/src/Masonry/getColumnCount.ts @@ -18,7 +18,6 @@ export default function getColumnCount({ minCols: number; layout: Layout; }): number { - console.log({ gutter }); if (layout === 'flexible' || (layout === 'serverRenderedFlexible' && width !== null)) { // "This is kind of crazy!" - you // Yes, indeed. The "guessing" here is meant to replicate the pass that the diff --git a/packages/gestalt/src/Masonry/multiColumnLayout.ts b/packages/gestalt/src/Masonry/multiColumnLayout.ts index 472a9419fa..6d733cbc09 100644 --- a/packages/gestalt/src/Masonry/multiColumnLayout.ts +++ b/packages/gestalt/src/Masonry/multiColumnLayout.ts @@ -548,7 +548,6 @@ function getPositionsWithMultiColumnItem({ ) || { itemsBatchSize: MULTI_COL_ITEMS_MEASURE_BATCH_SIZE, }; - console.log({ itemsBatchSize }); // Calculate how many items are on pre array and how many on graphBatch // pre items are positioned before the two column item diff --git a/packages/gestalt/src/Masonry/uniformRowLayout.test.ts b/packages/gestalt/src/Masonry/uniformRowLayout.test.ts index b65b4b8073..6f24cec4fb 100644 --- a/packages/gestalt/src/Masonry/uniformRowLayout.test.ts +++ b/packages/gestalt/src/Masonry/uniformRowLayout.test.ts @@ -1,3 +1,4 @@ +import { DEFAULT_LAYOUT_DEFAULT_GUTTER } from './getColumnCount'; import uniformRowLayout from './uniformRowLayout'; const stubCache = ( @@ -27,6 +28,7 @@ describe.each([false, true])('Uniform Row layout tests', (flexible) => { test('empty', () => { const layout = uniformRowLayout({ cache: stubCache(), + gutter: 0, width: 500, }); @@ -41,6 +43,7 @@ describe.each([false, true])('Uniform Row layout tests', (flexible) => { c: 100, }), flexible, + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, minCols: 2, width: 900, }); @@ -68,6 +71,7 @@ describe.each([false, true])('Uniform Row layout tests', (flexible) => { c: 100, }), flexible, + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, width: 250, minCols: 1, }); @@ -87,6 +91,7 @@ describe.each([false, true])('Uniform Row layout tests', (flexible) => { c: 100, }), flexible, + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, minCols: 2, width: 900, }); @@ -115,6 +120,7 @@ describe.each([false, true])('Uniform Row layout tests', (flexible) => { d: 100, }), flexible, + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, width: 800, }); @@ -147,6 +153,7 @@ describe('loadingStateItems', () => { const layout = uniformRowLayout({ cache: stubCache(), + gutter: DEFAULT_LAYOUT_DEFAULT_GUTTER, width: 500, renderLoadingState: true, }); From d9e4c8f8d47b65103ae85f095959c47312146e11 Mon Sep 17 00:00:00 2001 From: Bryam Rodriguez Date: Tue, 26 Nov 2024 11:59:46 -0600 Subject: [PATCH 5/5] Add iterations limit to DAG, configurable from outside --- .../masonry/MasonryContainer.tsx | 4 ---- packages/gestalt/src/Masonry/multiColumnLayout.ts | 15 +++++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/integration-test-helpers/masonry/MasonryContainer.tsx b/docs/integration-test-helpers/masonry/MasonryContainer.tsx index 36d7f36f32..7556e331de 100644 --- a/docs/integration-test-helpers/masonry/MasonryContainer.tsx +++ b/docs/integration-test-helpers/masonry/MasonryContainer.tsx @@ -420,10 +420,6 @@ export default class MasonryContainer extends Component>, const columnSpan = item.columnSpan as number | undefined; return columnSpan ?? 1; }} - _getModulePositioningConfig={() => ({ - itemsBatchSize: 6, - whitespaceThreshold: 32, - })} _logTwoColWhitespace={ logWhitespace ? // eslint-disable-next-line no-console diff --git a/packages/gestalt/src/Masonry/multiColumnLayout.ts b/packages/gestalt/src/Masonry/multiColumnLayout.ts index 6d733cbc09..7ac9f0d725 100644 --- a/packages/gestalt/src/Masonry/multiColumnLayout.ts +++ b/packages/gestalt/src/Masonry/multiColumnLayout.ts @@ -8,13 +8,17 @@ import { GetGraphPositionsReturn, NodeData, Position } from './types'; // This number can be dynamimcally set using _getModulePositioningConfig export const MULTI_COL_ITEMS_MEASURE_BATCH_SIZE = 5; +// We limit DAG iterations to 1MM to avoid running into obvious performance issues (or having the user waiting to much to see modules) +const DAG_ITERATIONS_HARD_LIMIT = 1000; + type GridSize = 'sm' | 'md' | 'lg' | 'xl'; export type ColumnSpanConfig = number | { [Size in GridSize]: number }; export type ModulePositioningConfig = { itemsBatchSize: number; // Maximum number of items used to position a module - whitespaceThreshold: number; // "Good enough" whitespace number when positioning a module + whitespaceThreshold?: number; // "Good enough" whitespace number when positioning a module + iterationsLimit?: number; }; // maps the number of columns to a grid breakpoint @@ -341,6 +345,7 @@ function getGraphPositions({ heights, whitespaceThreshold, columnSpan, + iterationsLimit = DAG_ITERATIONS_HARD_LIMIT, ...commonGetPositionArgs }: { items: ReadonlyArray; @@ -357,6 +362,7 @@ function getGraphPositions({ gutter: number; measurementCache: Cache; positionCache?: Cache; + iterationsLimit?: number; }): GetGraphPositionsReturn { // When whitespace threshold is set this variables store the score and node if found let bailoutScore; @@ -393,7 +399,7 @@ function getGraphPositions({ heightsArr: ReadonlyArray; itemsSoFar?: ReadonlyArray; }) { - if (bailoutNode) { + if (bailoutNode || numberOfIterations === iterationsLimit) { return; } @@ -542,7 +548,7 @@ function getPositionsWithMultiColumnItem({ // we need to fill those spaces with one col items const replaceWithOneColItems = !fitsFirstRow && multiColumnIndex < emptyColumns; - const { itemsBatchSize, whitespaceThreshold } = _getModulePositioningConfig?.( + const { itemsBatchSize, whitespaceThreshold, iterationsLimit } = _getModulePositioningConfig?.( columnCount, multiColumnItemColumnSpan, ) || { @@ -583,8 +589,9 @@ function getPositionsWithMultiColumnItem({ items: graphBatch, positions: paintedItemPositions, heights: paintedItemHeights, - whitespaceThreshold, columnSpan: multiColumnItemColumnSpan, + iterationsLimit, + whitespaceThreshold, ...commonGetPositionArgs, });