Skip to content

Commit 39d42c5

Browse files
committed
PR review
1 parent b93dd25 commit 39d42c5

3 files changed

Lines changed: 26 additions & 20 deletions

File tree

playwright.config.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { PlaywrightTestConfig } from "@playwright/test"
33
export default {
44
testDir: "integration-tests/",
55
testMatch: "**/*.spec.ts",
6-
// Platform-agnostic snapshots — headless Chromium renders identically on macOS and Linux
6+
// Platform-agnostic snapshot names — maxDiffPixels on each assertion handles minor
7+
// cross-platform rendering differences (font hinting, subpixel antialiasing).
78
snapshotPathTemplate: "{snapshotDir}/{testFileDir}/{testFileName}-snapshots/{arg}-{projectName}{ext}",
89
use: {
910
headless: true, // Always run headless to avoid disrupting work

src/components/charts/xy/ConnectedScatterplot.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,7 @@ export const ConnectedScatterplot = forwardRef(function ConnectedScatterplot<TDa
131131

132132
// Sort by orderAccessor if provided, and build a WeakMap of ordering
133133
// metadata so pointStyle can read the index directly without mutating user data.
134-
const orderMapRef = useRef(new WeakMap<Record<string, any>, { idx: number; total: number }>())
135-
const safeData = useMemo(() => {
134+
const { safeData, orderMap } = useMemo(() => {
136135
const xAcc = typeof xAccessor === "function" ? xAccessor : (d: any) => d[xAccessor]
137136
const yAcc = typeof yAccessor === "function" ? yAccessor : (d: any) => d[yAccessor]
138137
let sorted = rawData
@@ -162,8 +161,7 @@ export const ConnectedScatterplot = forwardRef(function ConnectedScatterplot<TDa
162161
map.set(d, { idx: idx++, total })
163162
}
164163
}
165-
orderMapRef.current = map
166-
return sorted
164+
return { safeData: sorted, orderMap: map }
167165
}, [rawData, orderAccessor, xAccessor, yAccessor])
168166

169167
// ── Dev-mode warnings ─────────────────────────────────────────────────
@@ -286,7 +284,7 @@ export const ConnectedScatterplot = forwardRef(function ConnectedScatterplot<TDa
286284

287285
const basePointStyle = useMemo(() => {
288286
return (d: Record<string, any>) => {
289-
const order = orderMapRef.current.get(d)
287+
const order = orderMap.get(d)
290288
const i = order?.idx ?? 0
291289
const n = order?.total ?? 1
292290
return {
@@ -297,7 +295,7 @@ export const ConnectedScatterplot = forwardRef(function ConnectedScatterplot<TDa
297295
fillOpacity: 1,
298296
}
299297
}
300-
}, [pointRadius])
298+
}, [pointRadius, orderMap])
301299

302300
const pointStyle = useMemo(
303301
() => wrapStyleWithSelection(basePointStyle, effectiveSelectionHook, selection),

src/components/server/componentSSR.test.tsx

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ function renderComponent(element: React.ReactElement): string {
8383
return ReactDOMServer.renderToStaticMarkup(element)
8484
}
8585

86+
function countPattern(html: string, pattern: RegExp): number {
87+
return (html.match(pattern) || []).length
88+
}
89+
8690
function countOccurrences(html: string, tag: string): number {
8791
const regex = new RegExp(`<${tag}[\\s/>]`, "g")
8892
return (html.match(regex) || []).length
@@ -924,9 +928,10 @@ describe("Component SSR — ConnectedScatterplot", () => {
924928
/>
925929
)
926930

927-
// svgPreRenderers should produce <line> elements connecting points
928-
// 5 points = 4 connecting segments
929-
expect(countOccurrences(html, "line")).toBeGreaterThanOrEqual(4)
931+
// Connecting segments use stroke-linecap="round" — unique to svgPreRenderer lines
932+
// 5 points = 4 connecting segments (each with stroke-linecap="round")
933+
const roundCapLines = countPattern(html, /stroke-linecap="round"/g)
934+
expect(roundCapLines).toBeGreaterThanOrEqual(4)
930935
})
931936

932937
it("renders halo lines when fewer than 100 points", () => {
@@ -940,10 +945,10 @@ describe("Component SSR — ConnectedScatterplot", () => {
940945
/>
941946
)
942947

943-
// Halo lines (white stroke) + segment lines = 8 total for 5 points
944-
// At minimum, more lines than just segments
945-
const lineCount = countOccurrences(html, "line")
946-
expect(lineCount).toBeGreaterThanOrEqual(8) // 4 halos + 4 segments
948+
// Halo lines have stroke="white" + stroke-linecap="round"
949+
// 5 points = 4 halos
950+
const haloLines = countPattern(html, /stroke="white"[^>]*stroke-linecap="round"/g)
951+
expect(haloLines).toBe(4)
947952
})
948953

949954
it("applies viridis colors to connecting segments", () => {
@@ -957,8 +962,9 @@ describe("Component SSR — ConnectedScatterplot", () => {
957962
/>
958963
)
959964

960-
// Viridis colors are hex strings like #440154 — segments should have stroke attributes
961-
expect(html).toMatch(/stroke="#[0-9a-f]{6}"/i)
965+
// Connecting segments have hex stroke + stroke-linecap="round" (unique to pre-renderer)
966+
const viridisSegments = countPattern(html, /stroke="#[0-9a-f]{6}"[^>]*stroke-linecap="round"/gi)
967+
expect(viridisSegments).toBe(4) // 4 connecting segments
962968
})
963969

964970
it("respects orderAccessor for sorting", () => {
@@ -980,9 +986,9 @@ describe("Component SSR — ConnectedScatterplot", () => {
980986
/>
981987
)
982988

983-
// Should still render circles and connecting lines
989+
// Should still render circles and round-capped connecting lines
984990
expect(countOccurrences(html, "circle")).toBeGreaterThanOrEqual(5)
985-
expect(countOccurrences(html, "line")).toBeGreaterThanOrEqual(4)
991+
expect(countPattern(html, /stroke-linecap="round"/g)).toBeGreaterThanOrEqual(4)
986992
})
987993
})
988994

@@ -1052,8 +1058,9 @@ describe("Component SSR — QuadrantChart", () => {
10521058
/>
10531059
)
10541060

1055-
// 2 center lines (vertical + horizontal)
1056-
expect(countOccurrences(html, "line")).toBeGreaterThanOrEqual(2)
1061+
// 2 center lines (vertical + horizontal) with default stroke="#999"
1062+
const centerLines = countPattern(html, /stroke="#999"/g)
1063+
expect(centerLines).toBe(2)
10571064
})
10581065

10591066
it("renders quadrant labels when showQuadrantLabels is true", () => {

0 commit comments

Comments
 (0)