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.

Sign up for GitHub

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

Jump to bottom

Masonry: Revert splitIndex change #3487

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Masonry: Revert splitIndex change #3487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
564e52c
[masonry] Revert splitIndex change
liuyenwei Mar 20, 2024
d0ccf02
update test
liuyenwei Mar 20, 2024
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
27 changes: 9 additions & 18 deletions packages/gestalt/src/Masonry/defaultTwoColumnModuleLayout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,6 @@ function calculateTwoColumnModuleWidth(columnWidth: number, gutter: number): num
return columnWidth * 2 + gutter;
}

function calculateSplitIndex<T: { +[string]: mixed }>(
itemsWithoutPositions: $ReadOnlyArray<T>,
): number {
// Currently we only support one two column item at the same time, more items will be supporped soon
const twoColumnIndex = itemsWithoutPositions.findIndex((item) => item.columnSpan === 2);

if (twoColumnIndex < TWO_COL_ITEMS_MEASURE_BATCH_SIZE) {
return 0;
}

if (twoColumnIndex + TWO_COL_ITEMS_MEASURE_BATCH_SIZE > itemsWithoutPositions.length) {
return itemsWithoutPositions.length - TWO_COL_ITEMS_MEASURE_BATCH_SIZE;
}

return twoColumnIndex;
}

function initializeHeightsArray<T>({
centerOffset,
columnCount,
Expand Down Expand Up @@ -302,8 +285,16 @@ const defaultTwoColumnModuleLayout = <T: { +[string]: mixed }>({
if (hasTwoColumnItems) {
// If the number of items to position is greater that the batch size
// we identify the batch with the two column item and apply the graph only to those items
const splitIndex = calculateSplitIndex(itemsWithoutPositions);
// Currently we only support one two column item at the same time, more items will be supporped soon
const twoColumnIndex = itemsWithoutPositions.indexOf(twoColumnItems[0]);

// If the number of items to position is greater that the batch size
// we identify the batch with the two column item and apply the graph only to those items
const shouldBatchItems = itemsWithoutPositions.length > TWO_COL_ITEMS_MEASURE_BATCH_SIZE;
const splitIndex =
twoColumnIndex + TWO_COL_ITEMS_MEASURE_BATCH_SIZE > itemsWithoutPositions.length
? itemsWithoutPositions.length - TWO_COL_ITEMS_MEASURE_BATCH_SIZE
: twoColumnIndex;
const pre = shouldBatchItems ? itemsWithoutPositions.slice(0, splitIndex) : [];
const batchWithTwoColumnItems = shouldBatchItems
? itemsWithoutPositions.slice(splitIndex, splitIndex + TWO_COL_ITEMS_MEASURE_BATCH_SIZE)
Expand Down
8 changes: 5 additions & 3 deletions packages/gestalt/src/Masonry/defaultTwoColumnModuleLayout.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ describe('two column layout test cases', () => {
]);
});

test('returns positions for all items when two columns item is on a large batch', () => {
test('correctly positions two column items regardless of on where they are in the batch', () => {
const measurementStore = new MeasurementStore<{ ... }, number>();
const positionCache = new MeasurementStore<{ ... }, Position>();
const heightsCache = new HeightsStore();
Expand Down Expand Up @@ -351,6 +351,7 @@ describe('two column layout test cases', () => {
measurementStore.set(item, item.height);
});
layout(mockItems);
// third row, first column
expect(positionCache.get(mockItems[twoColumnModuleIndex])).toEqual({
height: 200,
left: 99,
Expand All @@ -373,10 +374,11 @@ describe('two column layout test cases', () => {
measurementStore.set(item, item.height);
});
layout(mockItems);
// item 5 so second row, second column
expect(positionCache.get(mockItems[twoColumnModuleIndex])).toEqual({
height: 200,
left: 99,
top: 0,
left: 353,
top: 214,
width: 494,
});
});
Expand Down
Loading