Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 42
fix: preserve line continuity at zoom viewport edges#53
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
base:main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -18,6 +18,7 @@ import { | ||
| initialZoomWindow, | ||
| selectZoomRows, | ||
| visibleZoomData, | ||
| visibleZoomDataWithNeighbors, | ||
| zoomDateFromAnchor, | ||
| zoomDateKey, | ||
| zoomFullDomain, | ||
| @@ -69,19 +70,20 @@ const ZoomableTimeWindowExample = forwardRef< | ||
| }) | ||
| const [state, setState] = useState(stateRef.current) | ||
| const [scene, setScene] = useState<ChartScene<AaplRow> | null>(null) | ||
| const rows = visibleZoomData(zoomRows, state.window) | ||
| const visibleRows = visibleZoomData(zoomRows, state.window) | ||
| const lineRows = visibleZoomDataWithNeighbors(zoomRows, state.window) | ||
| const definition = useMemo( | ||
| () => | ||
| defineChart( | ||
| defineChart({ | ||
| marks: [ | ||
| lineY(rows, { | ||
| lineY(lineRows, { | ||
| x: 'Date', | ||
| y: 'Close', | ||
| stroke: color, | ||
| strokeWidth: 2.5, | ||
| }), | ||
| dot(rows, { | ||
| dot(visibleRows, { | ||
Comment on lines
+73
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
rg -n -C 8 'zoomGeometry|pointsBounds|geometry|role|visibleZoomData' \
benchmarks/conformanceRepository: TanStack/charts Length of output: 50372 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
echo"## target view.tsx relevant sections"
sed -n '1,180p' benchmarks/conformance/cases/90-zoomable-time-window/view.tsx
echo
sed -n '480,560p' benchmarks/conformance/cases/90-zoomable-time-window/view.tsx
echoecho"## local model exports"
sed -n '1,90p' benchmarks/conformance/cases/90-zoomable-time-window/model.ts
echoecho"## focused conformance files"
sed -n '20,30p' benchmarks/conformance/cases/90-zoomable-time-window/case.json
rg -n -C 4 'zoomGeometry|pointsBounds|geometry|role' benchmarks/conformance/cases/90-zoomable-time-window/view.tsxRepository: TanStack/charts Length of output: 13027 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
echo"## rest of target view.tsx"
sed -n '560,630p' benchmarks/conformance/cases/90-zoomable-time-window/view.tsx
echoecho"## behavioral simulation of point/window/data splits"
node - <<'JS'const zoomFullDomain = [ new Date(Date.UTC(2018, 0, 2)), new Date(Date.UTC(2018, 0, 18)),]const row = (dayOffset) => ({ Date: new Date(zoomFullDomain[0].getTime() + dayOffset * 86400_000), Close: dayOffset,})const zoomRows = Array.from({ length: 17 }, (_, i) => row(i))const window = { start: zoomRows[6].Date, end: zoomRows[8].Date }function visibleZoomData(rows, window) { const start = window.start.getTime() const end = window.end.getTime() return rows.filter((row) => { const timestamp = row.Date.getTime() return timestamp >= start && timestamp <= end })}function visibleZoomDataWithNeighbors(rows, window) { const start = window.start.getTime() const end = window.end.getTime() let firstVisible = -1 let lastVisible = -1 for (let index = 0; index < rows.length; index += 1) { const timestamp = rows[index].Date.getTime() if (timestamp < start) continue if (timestamp > end) break if (firstVisible < 0) firstVisible = index lastVisible = index } if (firstVisible < 0) return [] return rows.slice(Math.max(0, firstVisible - 1), Math.min(rows.length, lastVisible + 2))}function pointsBounds(points, bounds, scaleX, scaleY, color) { if (points.length === 0) return null let minX = Infinity, maxX = -Infinity, minY = Infinity, maxY = -Infinity const halfStroke = 1.25 for (const [px, py] of points) { minX = Math.min(minX, px) maxX = Math.max(maxX, px) minY = Math.min(minY, py) maxY = Math.max(maxY, py) } minX = Math.max(bounds.left, minX - halfStroke) maxX = Math.min(bounds.left + bounds.width, maxX + halfStroke) minY = Math.max(bounds.top, minY - halfStroke) maxY = Math.min(bounds.top + bounds.height, maxY + halfStroke) if (maxX - minX < 0 || maxY - minY < 0) return null return { x: minX, y: minY, width: maxX - minX, height: maxY - minY, paint: color }}const modelStart = new Date('2018-01-07T00:00:00Z')const modelEnd = new Date('2018-01-09T00:00:00Z')const bounds = { left: 58, top: 56, width: 450, height: 340 }const scaleX = bounds.width / bounds.widthconst scaleY = bounds.height / bounds.heightconst visible = visibleZoomData(zoomRows, window)const lineData = visibleZoomDataWithNeighbors(zoomRows, window)console.log(JSON.stringify({ windowRange: [window.start.toISOString().slice(0,10), modelEnd.toISOString().slice(0,10)], visibleDataDays: visible.map(r => r.Date.toISOString().slice(0,10)), lineDataDays: lineData.map(r => r.Date.toISOString().slice(0,10)), visiblePointsCount: visible.length, linePointsCount: lineData.length, visibleBoundsIfUsed: pointsBounds(visible.map(r => [r.Date.getTime(), r.Close]), bounds, scaleX, scaleY, 'red'), lineBoundsIfUsed: pointsBounds(lineData.map(r => [r.Date.getTime(), r.Close]), bounds, scaleX, scaleY, 'red'),}, null, 2))JSRepository: TanStack/charts Length of output: 1545 Use the rendered line data for line geometry.
🤖 Prompt for AI Agents | ||
| x: 'Date', | ||
| y: 'Close', | ||
| fill: color, | ||
| @@ -109,6 +111,7 @@ const ZoomableTimeWindowExample = forwardRef< | ||
| grid: true, | ||
| axis: { ticks: { count: 4 }, label: 'AAPL close ($)' }, | ||
| }, | ||
| clip: true, | ||
| margin: { top: 56, right: 24, bottom: 44, left: 58 }, | ||
| }), | ||
| { animate: false, keyboard: false, focus: focusDisabled }, | ||
There was a problem hiding this comment.
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
🧩 Analysis chain
🏁 Script executed:
Repository: TanStack/charts
Length of output: 50372
🏁 Script executed:
Repository: TanStack/charts
Length of output: 29091
Fix the neighbor scan to be order-agnostic.
visibleZoomDataselects rows by timestamp, but the newvisibleZoomDataWithNeighborsstops the scan aftertimestamp > end. That can miss out-of-order rows still inside the window and return neighbors for only a prefix of the selected rows. Update it to scan all rows from both sides/nearest rows, or keep it dependent on an explicit ascendingDateinvariant.🤖 Prompt for AI Agents