Skip to content
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

Masonry: Update fluid grid to not floor column width (take two) #3913

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion packages/gestalt/src/Masonry/fullWidthLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const fullWidthLayout = <T>({
// original implementation takes with CSS.
const colguess = Math.floor(width / idealColumnWidth);
const columnCount = Math.max(Math.floor((width - colguess * gutter) / idealColumnWidth), minCols);
const columnWidth = Math.floor(width / columnCount) - gutter;
const columnWidth = width / columnCount - gutter;
const columnWidthAndGutter = columnWidth + gutter;
const centerOffset = gutter / 2;

Expand Down
150 changes: 150 additions & 0 deletions packages/gestalt/src/Masonry/multiColumnLayout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1140,4 +1140,154 @@ describe('initializeHeightsArray', () => {
const newTops = newPositionsByColumns.map((column: any) => column[0].top);
expect(newTops).toEqual(heights);
});

test('correctly determines column heights before laying out new items (multi column layout w/ floating point column width)', () => {
const gutter = 16;
const columnWidth = 236.2942;
const measurementStore = new MeasurementStore<Record<any, any>, number>();
const positionCache = new MeasurementStore<Record<any, any>, Position>();
const items = [
{ name: 'Pin 0', height: 476 },
{ name: 'Pin 1', height: 381 },
{ name: 'Pin 2', height: 274 },
{ name: 'Pin 3', height: 303 },
{ name: 'Pin 4', height: 475 },
{ name: 'Pin 5', height: 496 },
{ name: 'Pin 6', height: 177 },
{ name: 'Pin 7', height: 440 },
{ name: 'Pin 8', height: 497 },
{ name: 'Pin 9', height: 430 },
{ name: 'Pin 10', height: 409 },
{ name: 'Pin 11', height: 452 },
{ name: 'Pin 12', height: 458 },
{ name: 'Pin 13', height: 510 },
{ name: 'Pin 14', height: 336 },
{ name: 'Pin 15', height: 293, columnSpan: 2 },
{ name: 'Pin 16', height: 416 },
{ name: 'Pin 17', height: 92 },
{ name: 'Pin 18', height: 475 },
{ name: 'Pin 19', height: 457 },
{ name: 'Pin 20', height: 300 },
{ name: 'Pin 21', height: 322 },
{ name: 'Pin 22', height: 417 },
];
items.forEach((item: any) => {
measurementStore.set(item, item.height);
});
const layout = (itemsToLayout: Item[]) =>
multiColumnLayout({
items: itemsToLayout,
gutter,
columnWidth,
columnCount: 9,
centerOffset: 1,
measurementCache: measurementStore,
positionCache,
_getColumnSpanConfig: getColumnSpanConfig,
});
const positions = layout(items);
expect(positions).toEqual([
{ top: 492, left: 1, width: 236.2942, height: 416 },
{ top: 512, left: 1262.471, width: 236.2942, height: 92 },
{ top: 513, left: 2019.3536, width: 236.2942, height: 300 },
{ top: 620, left: 1262.471, width: 236.2942, height: 475 },
{ top: 639, left: 1514.7651999999998, width: 236.2942, height: 457 },
{ top: 1112, left: 1262.471, width: 488.5884, height: 293 },
{ top: 0, left: 1, width: 236.2942, height: 476 },
{ top: 0, left: 253.2942, width: 236.2942, height: 381 },
{ top: 0, left: 505.5884, width: 236.2942, height: 274 },
{ top: 0, left: 757.8825999999999, width: 236.2942, height: 303 },
{ top: 0, left: 1010.1768, width: 236.2942, height: 475 },
{ top: 0, left: 1262.471, width: 236.2942, height: 496 },
{ top: 0, left: 1514.7651999999998, width: 236.2942, height: 177 },
{ top: 0, left: 1767.0593999999999, width: 236.2942, height: 440 },
{ top: 0, left: 2019.3536, width: 236.2942, height: 497 },
{ top: 193, left: 1514.7651999999998, width: 236.2942, height: 430 },
{ top: 290, left: 505.5884, width: 236.2942, height: 409 },
{ top: 319, left: 757.8825999999999, width: 236.2942, height: 452 },
{ top: 397, left: 253.2942, width: 236.2942, height: 458 },
{ top: 456, left: 1767.0593999999999, width: 236.2942, height: 510 },
{ top: 491, left: 1010.1768, width: 236.2942, height: 336 },
{ top: 715, left: 505.5884, width: 236.2942, height: 322 },
{ top: 787, left: 757.8825999999999, width: 236.2942, height: 417 },
]);

const heights = initializeHeightsArray({
centerOffset: 1,
columnCount: 9,
columnWidthAndGutter: columnWidth + gutter,
gutter,
items,
positionCache,
_getColumnSpanConfig: getColumnSpanConfig,
});

// calculate baseline heights using integer column width
const positionCacheInt = new MeasurementStore<Record<any, any>, Position>();
const layoutInt = (itemsToLayout: Item[]) =>
multiColumnLayout({
items: itemsToLayout,
gutter,
columnWidth: Math.floor(columnWidth),
columnCount: 9,
centerOffset: 1,
measurementCache: measurementStore,
positionCache: positionCacheInt,
_getColumnSpanConfig: getColumnSpanConfig,
});
layoutInt(items);
const heightsInt = initializeHeightsArray({
centerOffset: 1,
columnCount: 9,
columnWidthAndGutter: Math.floor(columnWidth) + gutter,
gutter,
items,
positionCache: positionCacheInt,
_getColumnSpanConfig: getColumnSpanConfig,
});

expect(heights.length).toEqual(9);
expect(heights).toEqual([924, 871, 1053, 1220, 843, 1421, 1421, 982, 829]);
expect(heights).toEqual(heightsInt);

const additionalItems = [
{ name: 'Pin 23', height: 428 },
{ name: 'Pin 24', height: 340 },
{ name: 'Pin 25', height: 458 },
{ name: 'Pin 26', height: 475 },
{ name: 'Pin 27', height: 303 },
{ name: 'Pin 28', height: 519 },
{ name: 'Pin 29', height: 440 },
{ name: 'Pin 30', height: 391 },
{ name: 'Pin 31', height: 475 },
{ name: 'Pin 32', height: 458 },
{ name: 'Pin 33', height: 292 },
{ name: 'Pin 34', height: 215 },
{ name: 'Pin 35', height: 400 },
{ name: 'Pin 36', height: 153 },
];
additionalItems.forEach((item: any) => {
measurementStore.set(item, item.height);
});

layout(items.concat(additionalItems));

const newPositions = additionalItems.map((item: any) => positionCache.get(item));

const newPositionsByColumns = newPositions
.reduce<Array<any>>((acc: any, position: any) => {
const column = Math.floor((position?.left ?? 0) / (columnWidth + gutter));
if (!acc[column]) {
acc[column] = [];
}
acc[column].push(position);
return acc;
}, [])
.map((column: any) => column.sort((a: any, b: any) => (a?.top ?? 0) - (b?.top ?? 0)));

// initializeHeights is run for each layout so this helps validate that running initializeHeights against the new set of items
// yields the correct positions based on the heights we validated above.
const newTops = newPositionsByColumns.map((column: any) => column[0].top);
expect(newTops).toEqual(heights);
});
});
4 changes: 3 additions & 1 deletion packages/gestalt/src/Masonry/multiColumnLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ export function initializeHeightsArray<T>({
items.forEach((item) => {
const position = positionCache?.get(item);
if (position) {
const col = (position.left - centerOffset) / columnWidthAndGutter;
// we do Math.round here because both position.left and columnWidthAndGutter can be floating point numbers
// in the case of fullWidthLayout (i.e. fluid grid)
const col = Math.round((position.left - centerOffset) / columnWidthAndGutter);
const columnSpan = calculateActualColumnSpan({ columnCount, item, _getColumnSpanConfig });
// the height of the column is just the sum of the top and height of the item
const absoluteHeight = position.top + position.height + gutter;
Expand Down
2 changes: 1 addition & 1 deletion playwright/masonry/flexible-resize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ test.describe('Masonry: flexible resize', () => {
const itemRectsAfter = await Promise.all(
gridItemsAfter.map((gridItemAfter) => gridItemAfter.boundingBox()),
);
expect(itemRectsAfter[0]?.width).toBe(273);
expect(Math.floor(itemRectsAfter[0]?.width)).toBe(273);
expect(itemRectsAfter[0]?.height).toBe(216);

// Get new sizes of grid items.
Expand Down
Loading