Skip to content

feat(dashboard): show a snap grid while dragging or resizing a tile#2715

Open
alex-fedotyev wants to merge 4 commits into
mainfrom
alex/dashboard-snap-grid
Open

feat(dashboard): show a snap grid while dragging or resizing a tile#2715
alex-fedotyev wants to merge 4 commits into
mainfrom
alex/dashboard-snap-grid

Conversation

@alex-fedotyev

Copy link
Copy Markdown
Contributor

Add a snap grid that appears while a dashboard tile is dragged or resized, so it is clear where the tile will land.

Summary

While a tile is being moved or resized, SnapGridLayout draws GridSnapOverlay behind the tiles: it outlines the grid cells the tile can snap to and brightens the cells around the tile's current position. It clears on drop. The overlay uses the same geometry react-grid-layout uses to place items, so the guides line up with where a tile actually lands, including the right and bottom edges (a naive line-per-column would sit one margin short there).

Changes

  • Add SnapGridLayout, a drop-in wrapper around WidthProvider(ReactGridLayout) that tracks the live drag/resize position and shows the overlay only while a tile is moving.
  • Add GridSnapOverlay, a non-interactive SVG overlay rendered behind the tiles (occupied cells stay covered, so only empty drop targets show through).
  • Add gridSnap.ts with the pure cell/neighbor geometry (computeSnapCells, computeSnapColWidth) plus unit tests.
  • Swap the two ReactGridLayout usages in DBDashboardPage for SnapGridLayout. Props are unchanged; the wrapper passes everything through.

Why

Dragging a tile on a dense dashboard gives no feedback about the grid it snaps to, so it is easy to misjudge where a tile will land. Outlining the actual snap cells (rather than lines on the column pitch) makes all four edges of the dropped tile land on a visible cell edge, and highlighting only the neighborhood keeps the rest of the grid faint so it does not compete with the tile content. The geometry lives in a pure helper so the alignment and neighbor math are unit tested.

Test plan

  • eslint + tsc on the changed files (clean)
  • unit: gridSnap.test.ts (column width, cell coverage, tile-edge alignment, neighbor detection); existing dashboard suites still pass
  • manual: drag and resize tiles on a dashboard; the grid appears on grab, tracks the tile, and clears on drop
  • light and dark themes

UI checks

  • Responsive: the overlay recomputes its cell geometry on container resize (ResizeObserver), so it stays aligned at any viewport width.
  • [ui-states: allow] It is a drag/resize affordance with no empty, loading, or error states of its own.
  • [no-story: allow] SnapGridLayout and GridSnapOverlay are grid wiring / overlay internals, not reusable UI components a Storybook story would exercise.

What is not in this PR

  • This is the first slice extracted from a larger dashboard metrics-UX prototype. The chart, legend, and tooltip changes are separate follow-up PRs.

Screenshots

snap grid, light

snap grid, dark

Draw the grid a tile snaps to while it is being dragged or resized, and
highlight the cells around the tile's current position so the drop target is
clear. The overlay is a drop-in wrapper (SnapGridLayout) around
WidthProvider(ReactGridLayout) that renders GridSnapOverlay during a
drag/resize and hides it on drop. Cell geometry mirrors react-grid-layout
(computeSnapCells), so the guides line up with where a tile actually lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 188cd73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jul 24, 2026 5:18pm
hyperdx-storybook Ready Ready Preview, Comment Jul 24, 2026 5:18pm

Request Review

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Diff size: 408 production lines changed (Tier 2 max: < 250)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 5
  • Production lines changed: 408 (+ 323 in test files, excluded from tier calculation)
  • Branch: alex/dashboard-snap-grid
  • Author: alex-fedotyev

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label Jul 23, 2026
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a dashboard grid wrapper that displays a measured SVG snap-cell overlay while tiles are dragged or resized.

  • Replaces both editable dashboard grid instances with SnapGridLayout.
  • Tracks the compacted landing position and clears the overlay after interaction.
  • Adds snap-cell geometry helpers, styling, unit tests, and a patch changeset.

Confidence Score: 5/5

The PR appears safe to merge with no blocking failures remaining.

No blocking failures remain.

Important Files Changed

Filename Overview
packages/app/src/DBDashboardPage.tsx Replaces the two existing editable dashboard grid instances with the snap-grid wrapper while preserving their props and children.
packages/app/src/components/dashboard/SnapGridLayout.tsx Wraps react-grid-layout callbacks to manage the overlay lifecycle and highlight the live landing footprint.
packages/app/src/components/dashboard/GridSnapOverlay.tsx Measures the grid container and renders a non-interactive SVG overlay behind dashboard tiles.
packages/app/src/components/dashboard/gridSnap.ts Computes snap-cell dimensions, positions, and focus-neighbor highlighting from react-grid-layout geometry.
packages/app/styles/GridSnapOverlay.module.scss Defines the overlay stacking, grid-cell appearance, highlighted state, and fade-in animation.

Reviews (4): Last reviewed commit: "Track the landing cell and fix the overl..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 242 passed • 1 skipped • 992s

Status Count
✅ Passed 242
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Environment note: The command sandbox (bwrap) failed to initialize for every shell call in this run (including sub-agents), so the multi-agent reviewer fan-out could not execute. This is a read-only manual review conducted by reading the changed files directly — gridSnap.ts, gridSnap.test.ts (src/__tests__/), SnapGridLayout.tsx, GridSnapOverlay.tsx, GridSnapOverlay.module.scss, and both DBDashboardPage.tsx call sites — across correctness, testing, maintainability, and performance lenses.

✅ No critical issues found. The ReactGridLayoutSnapGridLayout swap passes all props through unchanged, preserves the RGL.Layout typings, and the overlay reuses the grid's own cols/rowHeight/margin/containerPadding so its guides align with real drop positions. Phase-gating on the first move prevents a grid flash on plain clicks, and the geometry helper is well covered by gridSnap.test.ts.

🟡 P2 — recommended

  • packages/app/src/components/dashboard/SnapGridLayout.tsx:78 — The new drag/resize lifecycle logic (the makeHandler phase-gating, the placeholder ?? newItem landing choice, and the pointerup/pointercancel safety net) and GridSnapOverlay's ResizeObserver measurement have no test coverage; only the pure geometry in gridSnap.ts is exercised.
    • Fix: Add a component test asserting the overlay stays hidden on a plain click, appears only after the first move event, and is cleared on pointerup when no stop callback fires.
🔵 P3 nitpicks (2)
  • packages/app/src/components/dashboard/GridSnapOverlay.tsx:64computeSnapCells runs on every render and the full cell array re-renders each drag frame with no memoization, so a tall grid reconciles many <rect> nodes per frame while dragging.
    • Fix: Memoize the cell geometry that does not depend on focus (via useMemo keyed on size/cols/rowHeight/margin/containerPadding) so only the near flags recompute per frame.
  • packages/app/src/components/dashboard/SnapGridLayout.tsx:130 — The wrapper introduces an extra position: relative <div> around the grid, changing the DOM structure that previously placed ReactGridLayout as a direct child.
    • Fix: Confirm no dashboard styling targets the grid's former direct parent/sibling relationship, or note the added wrapper in the component doc comment.

Reviewers (0 of ~8 dispatched): multi-agent fan-out could not run — the sandbox failed to start any shell process, so reviewer sub-agents (correctness, testing, maintainability, performance, project-standards, agent-native, learnings) never executed. Findings above come from a manual read-only pass by the orchestrator applying those lenses directly.

Testing gaps: No tests cover SnapGridLayout's phase-gating, the pointerup/pointercancel teardown, or GridSnapOverlay's ResizeObserver measurement; gridSnap.test.ts covers only the pure cell/neighbor geometry.

Fix the failing lint: use a stable key for the snap cells (the array-index
key added the one warning that pushed the app over its max-warnings cap) and
rename the fade-in keyframe to kebab-case for stylelint.

Add the component tests the review asked for:
- SnapGridLayout drag/resize lifecycle: overlay mounts on start, focus
  tracks the moving tile, overlay clears on drop, and each caller callback
  still fires with its original arguments.
- GridSnapOverlay rendering: one rect per cell for the measured size, and
  the focus neighborhood gets the near class while the rest do not.
- A nonzero-containerPadding case for the geometry helper.

Maintainability nits: share the default margin/padding via gridSnap, return
colWidth from computeSnapCells so the overlay does not recompute it, memoize
the drag/resize handlers, and drop the redundant tuple casts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alex-fedotyev

Copy link
Copy Markdown
Contributor Author

Pushed ff70d23 to address the review.

Lint (the red check): the snap cells used an array-index key, which added the one warning that tipped the app package over its max-warnings cap. Switched to a stable key, and renamed the fade-in keyframe to kebab-case so stylelint passes too. nx run @hyperdx/app:ci:lint is green locally now.

P2 (tests): added component tests for both new pieces.

  • SnapGridLayout: fires the drag and resize callbacks and asserts the overlay mounts on start, focus tracks the moving tile, it clears on drop, and each caller callback still runs with its original arguments.
  • GridSnapOverlay: asserts one rect per snap cell for a measured size, and that the focus neighborhood gets the near class while the rest keep the base class.
  • Added a nonzero containerPadding case to the geometry helper test.

P3 nits: shared the default margin/padding through gridSnap, computeSnapCells now returns colWidth so the overlay does not recompute it, memoized the drag/resize handlers, and dropped the redundant tuple casts.

SnapGridLayout now imports FocusRect from gridSnap directly, so the
re-export from GridSnapOverlay is dead. Remove it to satisfy knip.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@teeohhem

Copy link
Copy Markdown
Contributor

@alex-fedotyev Very slick. Potential improvement: Is there a way to detect an invalid location for a tile and shade the boxes as red (or an obvious indicator that the placement wont work). See attached video.
My-Dashboard-–-ClickStack.webm

Addresses review feedback on the snap grid:

- The highlighted cells now follow where the tile will actually land
  (react-grid-layout's compacted placeholder position) instead of the raw
  cursor cell. Dragging a tile over a gap now shows it settling where vertical
  compaction will put it, not under the cursor where it won't stay.
- The overlay now appears only once a tile actually moves, not on the initial
  mousedown, so clicking a tile or its controls no longer flashes the grid or
  leaves it stuck on. A window pointerup/pointercancel safety net also tears it
  down if react-grid-layout skips its stop callback (e.g. the pointer is
  released over a menu that opened mid-drag).

Updates the SnapGridLayout tests for the new lifecycle: start without a move
never activates the overlay, pointer release clears it, and focus tracks the
landing rect rather than the cursor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alex-fedotyev

Copy link
Copy Markdown
Contributor Author

@teeohhem Good call, and thanks for the video, made the rough edge easy to see.

Digging in, there isn't really an "invalid" position to flag in this grid: react-grid-layout clamps a tile to the columns (you can't push it out left or right) and just grows the grid downward, and dragging over other tiles is meant to push them out of the way. So a red "won't work" state would almost never fire, and where it did it'd wrongly imply the move isn't allowed.

The real rough edge is the one in your video: when you drag a tile down over empty space, vertical compaction pulls it back up on release, but the highlight was following the cursor, so it looked like it would land somewhere it won't. Fixed that instead:

  • The highlighted cells now track where the tile will actually land (the compacted drop position), not the cursor. Drag into a gap and the highlight stays where it settles; drag under another tile and it shows it landing there as the grid grows.
  • While in there I also fixed a bug where the grid could stay stuck on: it now appears only once a tile actually moves (not on a plain click on a tile or its controls), and always clears on pointer release.

Pushed in 188cd73, give it another spin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants