Repository navigation
chore: tech polish — UrlInput state, notice a11y, table virtualization, TODO.md audit - #81
Conversation
…ns on notices - UrlInput is now a controlled component: the draft URL lives in App and is passed through ControlPanel, so collapsing the Data panel section (which unmounts its children) no longer wipes the typed text. The empty-state view shares the same draft, so it also survives the empty->loaded transition. - Notice banners (column-limit, empty-columns) now have role="status" + aria-live="polite" so screen readers announce them; the URL error banner already had role="alert". Added missing aria-labels on dismiss buttons and role="alert" on the URL validation error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0152hQSk9mV9zoZJBSRAJQWn
The full-dataset table view was capped at the first 1,000 rows (issue #56 / PR #60). Replace the cap and per-page pagination with simple, dependency-free windowed rendering: only the rows in the scroll viewport (plus a small overscan) are mounted, with top/bottom spacer rows standing in for the rest, so the FULL dataset (30k+ rows) is scrollable with a ~constant DOM row count. - computeRowWindow(scrollTop, viewportHeight, totalRows, rowHeight, overscan) -> {start, end, padTop, padBottom} lives in src/utils/tableLayout.ts as a pure, clamped function; capTableRows/TABLE_ROW_CAP are removed. - DataTable rows have a fixed height (TABLE_ROW_HEIGHT, whitespace-nowrap cells), the header is sticky, sorting scrolls back to the top, and the viewport height tracks the resizable panel via ResizeObserver. - App no longer caps tableRows or passes capNote. - Windowing math unit-tested (spacer/row-height invariant, end-of-data and shrunk-dataset clamping, overscroll, empty data, defaults). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0152hQSk9mV9zoZJBSRAJQWn
Audit verdicts (checked against the code):
- High 1 (non-destructive column filter): done — displayColumns derived via
filterColumns useMemo (B5).
- High 2 (vitest ^3.2.4 'not official'): stale — 3.2.4 is an official release.
- High 3/4 (selection/data hash robustness): done — full-sorted-set additive
hash + versioned dataStateHash feeding buildRenderKey.
- High 5 / Medium 3 (setTimeout cleanup / caching): done — RAF paint scheduler
with effect cleanup cancelling both rafId and timeoutId.
- Medium 1 (consolidate cache refs) / Low 1 (unused refs): stale — the old
triple-ref cache is gone (canvasElementsRef + ImageDataLRU snapshots now).
- Medium 2 (gridSize): done — single default parameter in selectionUtils.
- Low 2 (magic numbers): fixed here for the point radius — Canvas 2D path now
uses the shared POINT_RADIUS from webglPoints; padding/2 left as-is
(pervasive, self-explanatory, render-path churn not worth it).
- Low 3/4 ('@' alias -> ./src): declined — components/ and App.tsx live at the
repo root, so a src-rooted alias doesn't fit this layout; nothing uses '@/'.
- Low 5 (unused DndProvider/HTML5Backend imports): fixed here.
- Low 6 (filtering test re-implements app logic): fixed here — tests now call
the real filterColumns, incl. comma-OR semantics.
- Low 7 (non-null assertion on canvasContainerRef): done — guarded access.
- Low 8 (memoize visible count): stale — count is computed in App and passed
down as visibleDisplayCount.
- Random notes (duplicate spatial-grid callbacks, any[] typing): done.
Also updates the TODO.md references in PLAN.md/ROADMAP.md and corrects
CLAUDE.md's stale 'first 5 IDs' hash description.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0152hQSk9mV9zoZJBSRAJQWn
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview deployment
Published to |
…tech-polish Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0152hQSk9mV9zoZJBSRAJQWn
Bundle of the small known tech items from PLAN.md section 4.
What / why
1. UrlInput no longer loses the typed URL when the Data section collapses
PanelSectionunmounts its children while collapsed, so the URL draft held inUrlInputlocal state was wiped by a collapse/expand.UrlInputis now a controlled component: the draft lives inApp(urlDraft) and is passed throughControlPanel. Bonus: the draft also survives the empty-state → loaded transition, since bothUrlInputinstances share the same state.Guarantee: typecheck (the new required props make an unwired usage a compile error) + existing ControlPanel suites updated and green.
2. a11y live regions on the notice banners
The column-limit and empty-columns notices now carry
role="status"+aria-live="polite"so screen readers announce them (declined as out-of-scope in #67 triage, bundled here). The URL-error banner already hadrole="alert"(assertive live region — appropriate for an error), left as-is. Also added the missingaria-labelon the column-limit dismiss button androle="alert"on the URL validation error.Guarantee: covered by typecheck + existing suites (markup-only change).
3. Data table: windowed rendering, 1,000-row cap removed (noted in #60)
Dependency-free virtualization:
computeRowWindow(scrollTop, viewportHeight, totalRows, rowHeight, overscan) → {start, end, padTop, padBottom}insrc/utils/tableLayout.tspicks the visible row window;DataTablerenders only those rows between top/bottom spacer<tr>s, so the full dataset (30k+ rows) scrolls with a ~constant DOM row count (≈ viewport/37px + 2×5 overscan ≈ a few dozen<tr>s). Rows are fixed-height (TABLE_ROW_HEIGHT= 37px,whitespace-nowrapcells), the header is sticky, sorting jumps back to the top, and the viewport height tracks the resizable panel viaResizeObserver.capTableRows/TABLE_ROW_CAPand the page-based pagination are gone; the table look is otherwise unchanged.Guarantee: (a) — the windowing math is a pure clamped function with 12 unit tests in
src/test/tableLayout.test.ts(spacers+rows == full scroll height invariant, end-of-data clamping, stale scrollTop after dataset shrink, negative overscroll, empty data, partial rows, defaults, constant DOM row count). Perf conventions hold: per scroll frame the work is onesliceof ~30 rows — no O(N) DOM.4. TODO.md final audit → deleted
Every remaining item verified against the code:
displayColumnsderived viafilterColumnsuseMemo (B5)^3.2.4"not an official release"selectedStateHashfirst-5-IDs collision riskselectionUtils.ts)dataStateHashrobustnessdataStateHashfeedingbuildRenderKeysetTimeoutleak / caching-in-setTimeoutrafIdandtimeoutIdcanvasElementsRef+ImageDataLRUsnapshotsgridSizeselectionUtils.ts2.5point radius,padding/2)POINT_RADIUS);padding/2declined — pervasive, self-explanatory, render-path churn'@'alias →./src+ alias in testscomponents//App.tsxlive at the repo root; nothing imports via@/DndProvider/HTML5Backendtest importsfilterColumns, incl. a new comma-OR casecanvasContainerRefvisibleDisplayCountany[]typing)Nothing genuinely-undone remained to move to PLAN.md (only the pre-existing RAF-tuning line stays in section 4).
TODO.mddeleted; references updated in PLAN.md and ROADMAP.md; CLAUDE.md's stale "first 5 IDs" hash description corrected (it doesn't mention TODO.md).Gates
npm run typecheck✅npm run test:run✅ (31 files, 369 tests)npm run build✅🤖 Generated with Claude Code
https://claude.ai/code/session_0152hQSk9mV9zoZJBSRAJQWn