Skip to content
Open
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
98 changes: 98 additions & 0 deletions packages/core/src/layout/gridLayout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
export type TrackUnit = 'auto' | 'px' | 'percent' | 'fr';

export interface TrackSpec {
value: number;
unit: TrackUnit;
}

export interface GridCellRect {
x: number;
y: number;
width: number;
height: number;
}

export class GridLayoutManager {
/**
* Parse a track specification string like "100px 1fr 50% auto".
*/
static parseTrackSpec(specStr: string): TrackSpec[] {
const tokens = specStr.trim().split(/\s+/).filter(Boolean);
return tokens.map((token) => {
if (token === 'auto') return { value: 0, unit: 'auto' };
if (token.endsWith('px')) {
return { value: parseFloat(token.slice(0, -2)) || 0, unit: 'px' };
}
if (token.endsWith('%')) {
return { value: parseFloat(token.slice(0, -1)) || 0, unit: 'percent' };
}
if (token.endsWith('fr')) {
return { value: parseFloat(token.slice(0, -2)) || 1, unit: 'fr' };
}
const num = parseFloat(token);
return isNaN(num) ? { value: 0, unit: 'auto' } : { value: num, unit: 'px' };
Comment on lines +19 to +33

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟑 Minor | ⚑ Quick win

Preserve zero fr values and reject invalid factors.

parseTrackSpec('0fr') currently returns { value: 1, unit: 'fr' } because 0 || 1 is truthy fallback logic. This changes an explicit zero-flex track into a flexible track.

parseFloat also accepts numeric prefixes for malformed tokens. Since TrackSpec is public, callers can bypass the parser and pass negative or non-finite fr values. The allocation formula can then produce negative or NaN track sizes.

Use complete-token numeric parsing. Treat only the bare fr token as the default value 1. Validate TrackSpec.value before adding it to totalFr.

Proposed fix
-      if (token.endsWith('fr')) {
-        return { value: parseFloat(token.slice(0, -2)) || 1, unit: 'fr' };
-      }
+      if (token === 'fr') return { value: 1, unit: 'fr' };
+      if (token.endsWith('fr')) {
+        const value = Number(token.slice(0, -2));
+        if (!Number.isFinite(value) || value < 0) {
+          return { value: 0, unit: 'auto' };
+        }
+        return { value, unit: 'fr' };
+      }
...
       } else if (track.unit === 'fr') {
+        if (!Number.isFinite(track.value) || track.value < 0) {
+          throw new RangeError('fr track values must be finite and non-negative');
+        }
         totalFr += track.value;

Also applies to: 56-65

πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/layout/gridLayout.ts` around lines 19 - 33, Update
parseTrackSpec to use complete-token numeric parsing, preserve explicit 0fr, and
default only the bare fr token to value 1; malformed or invalid factors must not
be accepted from numeric prefixes. In the track allocation logic that
accumulates totalFr, validate each public TrackSpec value and ignore or safely
handle negative and non-finite fr values before adding them to totalFr.

});
}

/**
* Calculate track sizes given total container size and track specifications.
*/
static computeTrackSizes(totalSize: number, tracks: TrackSpec[]): number[] {
if (tracks.length === 0) return [];

const sizes = new Array<number>(tracks.length).fill(0);
let remainingSize = totalSize;
let totalFr = 0;

// First pass: allocate fixed px and percentage units
tracks.forEach((track, i) => {
if (track.unit === 'px') {
sizes[i] = Math.max(0, Math.min(remainingSize, track.value));
remainingSize -= sizes[i];
} else if (track.unit === 'percent') {
const allocated = Math.floor((totalSize * track.value) / 100);
sizes[i] = Math.max(0, Math.min(remainingSize, allocated));
remainingSize -= sizes[i];
} else if (track.unit === 'fr') {
totalFr += track.value;
}
});

// Second pass: allocate fr units from remaining size
if (totalFr > 0 && remainingSize > 0) {
tracks.forEach((track, i) => {
if (track.unit === 'fr') {
sizes[i] = Math.floor((remainingSize * track.value) / totalFr);
}
});
}
Comment on lines +61 to +68

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟑 Minor | ⚑ Quick win

Distribute the fractional rounding remainder.

Independent Math.floor calls can leave part of the available space unused. For example, three 1fr tracks in a size of 10 return [3, 3, 3]. The cell rectangles then cover only 9 cells.

Allocate the remainder to one fractional track or use a largest-remainder distribution. Add a test that verifies the fractional sizes sum to the remaining space.

Proposed fix
+      const frIndexes = tracks.flatMap((track, index) =>
+        track.unit === 'fr' && track.value > 0 ? [index] : [],
+      );
+      let allocatedFrSize = 0;
       tracks.forEach((track, i) => {
         if (track.unit === 'fr') {
-          sizes[i] = Math.floor((remainingSize * track.value) / totalFr);
+          sizes[i] =
+            i === frIndexes[frIndexes.length - 1]
+              ? remainingSize - allocatedFrSize
+              : Math.floor((remainingSize * track.value) / totalFr);
+          allocatedFrSize += sizes[i];
         }
       });
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Second pass: allocate fr units from remaining size
if (totalFr > 0 && remainingSize > 0) {
tracks.forEach((track, i) => {
if (track.unit === 'fr') {
sizes[i] = Math.floor((remainingSize * track.value) / totalFr);
}
});
}
// Second pass: allocate fr units from remaining size
if (totalFr > 0 && remainingSize > 0) {
const frIndexes = tracks.flatMap((track, index) =>
track.unit === 'fr' && track.value > 0 ? [index] : [],
);
let allocatedFrSize = 0;
tracks.forEach((track, i) => {
if (track.unit === 'fr') {
sizes[i] =
i === frIndexes[frIndexes.length - 1]
? remainingSize - allocatedFrSize
: Math.floor((remainingSize * track.value) / totalFr);
allocatedFrSize += sizes[i];
}
});
}
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/layout/gridLayout.ts` around lines 61 - 68, Update the
second-pass fractional allocation in the grid layout calculation so integer
rounding distributes all leftover units among the `fr` tracks, ensuring their
sizes sum to `remainingSize` (for example, three 1fr tracks over 10 units must
total 10). Add a test covering fractional tracks and assert that the computed
sizes sum to the available remaining space.


return sizes;
}

/**
* Calculate cell rectangle for a given row and column index in the grid.
*/
static getCellRect(
colIndex: number,
rowIndex: number,
colSizes: number[],
rowSizes: number[],
startPoint: { x: number; y: number } = { x: 0, y: 0 }
): GridCellRect {
let x = startPoint.x;
for (let i = 0; i < colIndex && i < colSizes.length; i++) {
x += colSizes[i];
}

let y = startPoint.y;
for (let j = 0; j < rowIndex && j < rowSizes.length; j++) {
y += rowSizes[j];
}

const width = colSizes[colIndex] ?? 0;
const height = rowSizes[rowIndex] ?? 0;

return { x, y, width, height };
}
}
40 changes: 40 additions & 0 deletions packages/core/test/gridLayout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, it, expect } from 'bun:test';
import { GridLayoutManager } from '../src/layout/gridLayout';

describe('GridLayoutManager Unit Tests', () => {
it('should parse track specifications correctly', () => {
const tracks = GridLayoutManager.parseTrackSpec('100px 1fr 50% auto');
expect(tracks).toEqual([
{ value: 100, unit: 'px' },
{ value: 1, unit: 'fr' },
{ value: 50, unit: 'percent' },
{ value: 0, unit: 'auto' },
]);
});

it('should compute track sizes for mixed px, percent, and fr tracks', () => {
const tracks = GridLayoutManager.parseTrackSpec('20px 50% 1fr');
// Total 100px. Fixed 20px, 50% = 50px. Remaining = 30px -> 1fr gets 30px.
const sizes = GridLayoutManager.computeTrackSizes(100, tracks);
expect(sizes).toEqual([20, 50, 30]);
});

it('should compute equal track sizes for multiple fr tracks', () => {
const tracks = GridLayoutManager.parseTrackSpec('1fr 2fr');
const sizes = GridLayoutManager.computeTrackSizes(90, tracks);
expect(sizes).toEqual([30, 60]);
});
Comment on lines +22 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟑 Minor | ⚑ Quick win

Cover non-divisible fr allocations.

Lines 22-26 use a divisible total, so the test does not detect rounding gaps. For a total size of 100 with 1fr 1fr 1fr, computeTrackSizes returns [33, 33, 33], and the tracks cover only 99 cells.

Add a regression case and distribute the remainder, or define an explicit remainder policy, before relying on this test for arbitrary terminal sizes.

Suggested regression test
+  it('should fill the container when fr tracks require rounding', () => {
+    const tracks = GridLayoutManager.parseTrackSpec('1fr 1fr 1fr');
+    const sizes = GridLayoutManager.computeTrackSizes(100, tracks);
+
+    expect(sizes.reduce((total, size) => total + size, 0)).toBe(100);
+  });
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/test/gridLayout.test.ts` around lines 22 - 26, Update
computeTrackSizes and its related fr-allocation logic to handle non-divisible
totals without leaving uncovered cells; for a total of 100 with three 1fr
tracks, ensure the implementation follows an explicit remainder policy and the
resulting sizes cover all 100 cells. Add a regression test alongside the
existing GridLayoutManager track-size tests for this case.


it('should calculate correct cell rectangle coordinates', () => {
const colSizes = [20, 30, 50];
const rowSizes = [10, 15];

const rect = GridLayoutManager.getCellRect(1, 1, colSizes, rowSizes, { x: 5, y: 5 });
expect(rect).toEqual({
x: 25, // 5 + 20
y: 15, // 5 + 10
width: 30,
height: 15,
});
});
});
Loading