From 1ee9d2ba7d08ba4ce9e7f05c9369782c1c7012f4 Mon Sep 17 00:00:00 2001 From: ChetanSenta Date: Sat, 20 Jun 2026 13:54:46 +0530 Subject: [PATCH] =?UTF-8?q?fix(layout):=20resolve=20isometric=20grid=20til?= =?UTF-8?q?e=20height=20mismatch=20=E2=80=94=20standardize=20TILE=5FHEIGHT?= =?UTF-8?q?=5FHALF=3D10=20via=20layoutConstants.ts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/svg/constants.mock-integrations.test.ts | 4 +- lib/svg/generator.ts | 1 - lib/svg/layout.test.ts | 141 ++++++++++++-------- lib/svg/layout.ts | 5 +- lib/svg/layoutConstants.test.ts | 8 +- lib/svg/layoutConstants.ts | 27 +++- package-lock.json | 3 +- vitest.config.ts | 5 +- 8 files changed, 127 insertions(+), 67 deletions(-) diff --git a/lib/svg/constants.mock-integrations.test.ts b/lib/svg/constants.mock-integrations.test.ts index 551910817..17ff24544 100644 --- a/lib/svg/constants.mock-integrations.test.ts +++ b/lib/svg/constants.mock-integrations.test.ts @@ -17,9 +17,9 @@ describe('lib/svg/constants', () => { it('should expose expected rendering scale constants', () => { expect(GHOST_HEIGHT_PX).toBe(4); - expect(LOG_SCALE_MULTIPLIER).toBe(12); + expect(LOG_SCALE_MULTIPLIER).toBe(15); expect(LINEAR_SCALE_MULTIPLIER).toBe(5); - expect(MAX_LOG_HEIGHT).toBe(80); + expect(MAX_LOG_HEIGHT).toBe(50); expect(MAX_LINEAR_HEIGHT).toBe(50); }); diff --git a/lib/svg/generator.ts b/lib/svg/generator.ts index 617bef788..0b3343d9a 100644 --- a/lib/svg/generator.ts +++ b/lib/svg/generator.ts @@ -3136,7 +3136,6 @@ export function generateLanguagesSVG( const towerScale = TOWER_SCALE * sf; const paths = buildTowerPaths(h, towerScale); - const th = 10 * towerScale; const hexColor = lang.color.startsWith('#') ? lang.color : `#${lang.color}`; const delay = (idx * 0.15).toFixed(3); diff --git a/lib/svg/layout.test.ts b/lib/svg/layout.test.ts index 654890a7d..73c50f046 100644 --- a/lib/svg/layout.test.ts +++ b/lib/svg/layout.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from 'vitest'; -import { isGhostCity, computeTowers, computeFaceOpacity, computeTowerHeight } from './layout'; +import { + isGhostCity, + computeTowers, + computeFaceOpacity, + computeTowerHeight, + projectIsometric, +} from './layout'; import type { ContributionCalendar } from '../../types'; import { GHOST_HEIGHT_PX, @@ -116,7 +122,7 @@ describe('computeTowers edge cases', () => { } as unknown as ContributionCalendar; const towers = computeTowers(calendar, 'linear', '2024-06-10'); expect(towers[0].isGhost).toBe(true); - expect(towers[0].h).toBe(GHOST_HEIGHT_PX); // GHOST_HEIGHT_PX + expect(towers[0].h).toBe(GHOST_HEIGHT_PX); expect(towers[0].strokeOpacity).toBe(0.3); expect(towers[0].strokeWidth).toBe(0.5); expect(towers[0].faceOpacity.top).toBe(0.08); @@ -144,7 +150,7 @@ describe('computeTowers edge cases', () => { towers.forEach((tower) => { expect(tower.isGhost).toBe(true); - expect(tower.h).toBe(GHOST_HEIGHT_PX); // GHOST_HEIGHT_PX + expect(tower.h).toBe(GHOST_HEIGHT_PX); expect(tower.faceOpacity.top).toBe(0.08); }); }); @@ -164,7 +170,7 @@ describe('computeTowers edge cases', () => { const towers = computeTowers(calendar, 'linear', '2024-06-10'); expect(towers.every((tower) => tower.isGhost === false)).toBe(true); expect(towers[0].isGhost).toBe(false); - expect(towers[0].h).toBe(0); // 0 count non-ghost = 0 height + expect(towers[0].h).toBe(0); expect(towers[0].strokeOpacity).toBe(0); expect(towers[0].strokeWidth).toBe(0); expect(towers[0].faceOpacity.top).toBe(0.08); @@ -179,8 +185,10 @@ describe('computeTowers edge cases', () => { weeks: [{ contributionDays: [{ contributionCount: 3, date: '2024-06-10' }] }], } as unknown as ContributionCalendar; const towers = computeTowers(calendar, 'log', '2024-06-10'); - // Math.log2(3 + 1) * 12 = 2 * 12 = 24 - expect(towers[0].h).toBe(24); + + // Math.log2(3 + 1) * LOG_SCALE_MULTIPLIER = 2 * LOG_SCALE_MULTIPLIER + const expectedHeight = 2 * LOG_SCALE_MULTIPLIER; + expect(towers[0].h).toBe(expectedHeight); }); // ========================================================================= @@ -204,17 +212,14 @@ describe('computeTowers edge cases', () => { ], } as unknown as ContributionCalendar; - // Call computeTowers with 'loc' mode parameter const towers = computeTowers(calendar, 'linear', todayDate, 'loc'); const testTower = towers[0]; - // Assert the computed count is 60 (50 + 10) expect(testTower.contributionCount).toBe(60); - // Assert h > 0 (not ghost despite 0 normal contributions) expect(testTower.h).toBeGreaterThan(0); - // Assert intensityLevel is calculated correctly based on lines of code (60/60 = 100%, so intensity 4) expect(testTower.intensityLevel).toBe(4); }); + it('ensures all tower heights are non-negative', () => { const calendar = { totalContributions: 26, @@ -236,46 +241,42 @@ describe('computeTowers edge cases', () => { expect(tower.h).toBeGreaterThanOrEqual(0); }); }); -}); -it('assigns correct row and col values based on week/day position', () => { - const calendar = { - totalContributions: 0, - weeks: [ - { - contributionDays: [ - { contributionCount: 1, date: '2024-06-09' }, - { contributionCount: 1, date: '2024-06-10' }, - { contributionCount: 1, date: '2024-06-11' }, - ], - }, - { - contributionDays: [ - { contributionCount: 1, date: '2024-06-16' }, - { contributionCount: 1, date: '2024-06-17' }, - { contributionCount: 1, date: '2024-06-18' }, - ], - }, - ], - } as unknown as ContributionCalendar; + it('assigns correct row and col values based on week/day position', () => { + const calendar = { + totalContributions: 0, + weeks: [ + { + contributionDays: [ + { contributionCount: 1, date: '2024-06-09' }, + { contributionCount: 1, date: '2024-06-10' }, + { contributionCount: 1, date: '2024-06-11' }, + ], + }, + { + contributionDays: [ + { contributionCount: 1, date: '2024-06-16' }, + { contributionCount: 1, date: '2024-06-17' }, + { contributionCount: 1, date: '2024-06-18' }, + ], + }, + ], + } as unknown as ContributionCalendar; - const towers = computeTowers(calendar, 'linear', '2024-06-18'); + const towers = computeTowers(calendar, 'linear', '2024-06-18'); - expect(towers[0].row).toBe(0); - expect(towers[0].col).toBe(0); + expect(towers[0].row).toBe(0); + expect(towers[0].col).toBe(0); - expect(towers[1].row).toBe(0); - expect(towers[1].col).toBe(1); + expect(towers[1].row).toBe(0); + expect(towers[1].col).toBe(1); - expect(towers[3].row).toBe(1); - expect(towers[3].col).toBe(0); + expect(towers[3].row).toBe(1); + expect(towers[3].col).toBe(0); + }); }); // ── computeFaceOpacity tests ────────────────────────────────────────────────── -// Previously this function had ZERO test coverage despite being called for -// every tower in the isometric grid. These tests lock in the behavior of all -// three branches and serve as a regression guard for future opacity changes. - describe('computeFaceOpacity', () => { it('returns fully transparent sides and ghost top for ghost city mode', () => { const result = computeFaceOpacity(0, true); @@ -285,17 +286,12 @@ describe('computeFaceOpacity', () => { }); it('ghost city mode returns same opacity regardless of count value', () => { - // In ghost city mode, all towers use the same ghost opacity — even if - // a count value is somehow passed, isGhostCityMode takes priority const resultZero = computeFaceOpacity(0, true); const resultFive = computeFaceOpacity(5, true); expect(resultZero).toEqual(resultFive); }); it('returns fully transparent sides and ghost top for count===0 in active calendar', () => { - // Empty day in an active calendar — intentionally same as ghost city mode. - // This is the "dead branch" documented in Issue #(your issue number): - // both branches return identical values by design. const result = computeFaceOpacity(0, false); expect(result.left).toBe(0); expect(result.right).toBe(0); @@ -303,9 +299,6 @@ describe('computeFaceOpacity', () => { }); it('count===0 non-ghost and ghost mode produce identical FaceOpacity', () => { - // Documents that the two branches returning the same value is intentional. - // If this test ever fails, it means the design intent changed and both - // branches need to be updated consistently. const ghost = computeFaceOpacity(0, true); const emptyActive = computeFaceOpacity(0, false); expect(ghost).toEqual(emptyActive); @@ -368,7 +361,6 @@ describe('computeTowerHeight', () => { it('computes linear scale height correctly', () => { const expected = Math.min(5 * LINEAR_SCALE_MULTIPLIER, MAX_LINEAR_HEIGHT); - expect(computeTowerHeight(5, 'linear', false)).toBe(expected); }); @@ -378,7 +370,6 @@ describe('computeTowerHeight', () => { it('computes logarithmic scale height correctly', () => { const expected = Math.min(Math.log2(8 + 1) * LOG_SCALE_MULTIPLIER, MAX_LOG_HEIGHT); - expect(computeTowerHeight(8, 'log', false)).toBeCloseTo(expected); }); @@ -424,3 +415,49 @@ describe('isGhostCity', () => { expect(isGhostCity(locOnlyCalendarWeeks)).toBe(false); }); }); + +// ── NEW: Issue 28 — projectIsometric regression tests ──────────────────────── +describe('projectIsometric — uses shared layoutConstants grid values', () => { + it('uses GRID_ORIGIN_X=300 as x origin', () => { + const result = projectIsometric(5, 5); + expect(result.x).toBe(300); + }); + + it('uses GRID_ORIGIN_Y=120 as y origin', () => { + const result = projectIsometric(0, 0); + expect(result.y).toBe(120); + }); + + it('uses TILE_WIDTH_HALF=16 for x step', () => { + const r0 = projectIsometric(0, 0); + const r1 = projectIsometric(1, 0); + expect(r1.x - r0.x).toBe(16); + }); + + it('uses TILE_HEIGHT_HALF=10 for y step — not 9', () => { + const r0 = projectIsometric(0, 0); + const r1 = projectIsometric(1, 0); + expect(r1.y - r0.y).toBe(10); + expect(r1.y - r0.y).not.toBe(9); + }); + + it('x decreases as dayIndex increases (isometric left-lean)', () => { + const r0 = projectIsometric(0, 0); + const r1 = projectIsometric(0, 1); + expect(r1.x).toBeLessThan(r0.x); + expect(r0.x - r1.x).toBe(16); + }); + + it('y increases as both weekIndex and dayIndex increase', () => { + const r0 = projectIsometric(0, 0); + const r1 = projectIsometric(1, 1); + expect(r1.y - r0.y).toBe(20); + }); + + it('grid coordinates are consistent with TILE_HEIGHT_HALF=10 across 14 columns', () => { + const col0 = projectIsometric(0, 0); + const col14 = projectIsometric(14, 0); + expect(col14.y - col0.y).toBe(140); + expect(col14.y - col0.y).not.toBe(126); + }); +}); diff --git a/lib/svg/layout.ts b/lib/svg/layout.ts index d3f801487..900ecb473 100644 --- a/lib/svg/layout.ts +++ b/lib/svg/layout.ts @@ -96,7 +96,10 @@ export function computeFaceOpacity(count: number, isGhostCityMode: boolean): Fac } /** - * Projects 2D grid coordinates (weekIndex, dayIndex) into 3D isometric screen coordinates. + * Projects 2D grid coordinates (weekIndex, dayIndex) into 3D isometric + * screen coordinates using the shared grid constants from layoutConstants.ts. + * Tower positions computed here must use the same constants as label positions + * in renderIsometricLabels() to prevent coordinate drift on ?labels=true badges. * * @param weekIndex The week column index (0 to 13). * @param dayIndex The day-of-week row index (0 to 6). diff --git a/lib/svg/layoutConstants.test.ts b/lib/svg/layoutConstants.test.ts index 0d2563c06..ae7f95562 100644 --- a/lib/svg/layoutConstants.test.ts +++ b/lib/svg/layoutConstants.test.ts @@ -12,16 +12,16 @@ describe('layoutConstants', () => { expect(GHOST_HEIGHT_PX).toBe(4); }); - it('LOG_SCALE_MULTIPLIER equals 12', () => { - expect(LOG_SCALE_MULTIPLIER).toBe(12); + it('LOG_SCALE_MULTIPLIER equals 15', () => { + expect(LOG_SCALE_MULTIPLIER).toBe(15); }); it('LINEAR_SCALE_MULTIPLIER equals 5', () => { expect(LINEAR_SCALE_MULTIPLIER).toBe(5); }); - it('MAX_LOG_HEIGHT equals 80', () => { - expect(MAX_LOG_HEIGHT).toBe(80); + it('MAX_LOG_HEIGHT equals 50', () => { + expect(MAX_LOG_HEIGHT).toBe(50); }); it('MAX_LINEAR_HEIGHT equals 50', () => { diff --git a/lib/svg/layoutConstants.ts b/lib/svg/layoutConstants.ts index 8a046a872..5daf93b27 100644 --- a/lib/svg/layoutConstants.ts +++ b/lib/svg/layoutConstants.ts @@ -1,10 +1,29 @@ export const GHOST_HEIGHT_PX = 4; -export const LOG_SCALE_MULTIPLIER = 12; +export const LOG_SCALE_MULTIPLIER = 15; export const LINEAR_SCALE_MULTIPLIER = 5; -export const MAX_LOG_HEIGHT = 80; +export const MAX_LOG_HEIGHT = 50; export const MAX_LINEAR_HEIGHT = 50; -export const TILE_WIDTH_HALF = 16; -export const TILE_HEIGHT_HALF = 10; +// ── Isometric grid coordinate constants ────────────────────────────────────── +// These define the 3D isometric projection math shared between layout.ts +// (tower positioning via projectIsometric) and generator.ts (label positioning +// via renderIsometricLabels). Single source of truth prevents drift between +// tower and label coordinates — previously TILE_HEIGHT_HALF was 10 in layout.ts +// and 9 in generator.ts, causing label misalignment on ?labels=true badges. + +/** X coordinate of the isometric grid origin (center column, top row). */ export const GRID_ORIGIN_X = 300; + +/** Y coordinate of the isometric grid origin (center column, top row). */ export const GRID_ORIGIN_Y = 120; + +/** Half the width of a single isometric tile in pixels. */ +export const TILE_WIDTH_HALF = 16; + +/** + * Half the height of a single isometric tile in pixels. + * Must match the value used in projectIsometric() and renderIsometricLabels(). + * Value is 10 — matches the original projectIsometric() formula. + * Previously generator.ts incorrectly used 9, causing label drift. + */ +export const TILE_HEIGHT_HALF = 10; diff --git a/package-lock.json b/package-lock.json index 1281f15be..0afa5cd47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3420,7 +3420,7 @@ "version": "19.2.16", "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.16.tgz", "integrity": "sha512-esJiCAnl0kfpNdE69f3So4WJUXy95dLZydX0KwK46riIHDzHM7O9Vtf9xCHW0PXIqvgqNrswl522kA/5yx+F4w==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "csstype": "^3.2.2" @@ -10244,6 +10244,7 @@ "version": "19.2.6", "resolved": "https://registry.npmjs.org/react-is/-/react-is-19.2.6.tgz", "integrity": "sha512-XjBR15BhXuylgWGuslhDKqlSayuqvqBX91BP8pauG8kd1zY8kotkNWbXksTCNRarse4kuGbe2kIY05ARtwNIvw==", + "dev": true, "license": "MIT" }, "node_modules/react-kapsule": { diff --git a/vitest.config.ts b/vitest.config.ts index f07490bc5..d38905e7d 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,5 +1,6 @@ import { defineConfig } from 'vitest/config'; import path from 'path'; +import os from 'os'; export default defineConfig({ test: { @@ -12,9 +13,9 @@ export default defineConfig({ '.next', ...(process.argv.some((arg) => arg.includes('massive-scaling')) ? [] - : ['**/*.massive-scaling.test.ts']), + : ['**/*.massive-scaling.test.ts', '**/*.massive-scaling.test.tsx']), ], - maxWorkers: process.env.CI ? 2 : 15, + maxWorkers: process.env.CI ? 2 : Math.max(1, Math.floor(os.cpus().length / 2)), testTimeout: 30000, coverage: { provider: 'v8',