Skip to content

PR-4: dashboards editor single-tap action menu (dashboard editor gesture alignment)#200

Merged
mairas merged 1 commit into
mainfrom
feat/dashboards-editor-single-tap
Jul 4, 2026
Merged

PR-4: dashboards editor single-tap action menu (dashboard editor gesture alignment)#200
mairas merged 1 commit into
mainfrom
feat/dashboards-editor-single-tap

Conversation

@mairas

@mairas mairas commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

PR-4 of the navigation UX redesign (#183) — the last sub-issue. The Settings → Dashboards editor was the one surface still using the retired gesture vocabulary: double-tap a page tile to edit, long-press for a Duplicate/Delete bottom sheet. This brings it in line with the rest of the redesign.

What changes

  • Single tap on a page tile → one responsive action menu (Edit / Duplicate / Delete) at the tap point, reusing the same ActionMenuComponent the widget grid uses (mat-menu on desktop, bottom drawer under 600px).
  • Delete now confirms via a dialog before removing the page and its widgets (previously the bottom sheet deleted immediately).
  • The bespoke DashboardsBottomSheetComponent, the press/drag-arbitration state it needed (_sheetOpen, suppressDrag, cancelPointerSequence, …), and two dead fields (pageTitle, DomSanitizer) are removed.

Tap vs. drag

The tiles are cdkDrag reorderable and now also carry kipGestures (tap). Tap-vs-drag is left entirely to the gesture directive's movement threshold: a real reorder moves past the tap slop (20px touch / 8px mouse) and emits no tap, so it never opens the menu. There is deliberately no isDragging guard on the tap handler — gating on the shared drag signal would swallow a legitimate tap whose minor travel already tripped cdkDrag's lower 5px start threshold (common on a rocking marine display), which review flagged as a real defect.

Review

Five independent persona reviewers (correctness, adversarial, maintainability, testing, project-standards). One genuine defect (the tap-swallow above) fixed; the private-visibility and test-coverage findings applied; no standards violations. The positional _menuIndex capture and the app-global isDragging signal were noted as residual risks — both safe today (the menu is modal; dashboards() has no live async mutation) and out of scope to re-architect here.

Verification: lint clean, build:prod green, editor spec 10/10 locally (menu contents, single-tap + keyboard open, edit/duplicate apply-on-save, delete-confirms-and-only-on-confirm, stale-index guard). CI runs the full matrix.

Closes #188. Refs #183.

The Settings → Dashboards editor still used the retired gesture vocabulary
— double-tap a tile to edit, long-press for a Duplicate/Delete bottom
sheet. Bring it in line with the rest of the redesign: a single tap on a
page tile opens one responsive ActionMenuComponent (Edit / Duplicate /
Delete) at the tap point, reusing the same menu the widget grid uses.

Delete now routes through a confirmation dialog before removing the page
and its widgets. The bespoke DashboardsBottomSheetComponent and the
press/drag-arbitration state it needed are removed, along with two dead
fields (pageTitle, DomSanitizer).

Closes #188. Refs #183.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDu7zsKECA9SvBR5DLdUex
@mairas

mairas commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Review synthesis

Five independent persona reviewers (correctness, adversarial, maintainability, testing, project-standards), each in a fresh context.

Confirmed and fixed

# Finding Severity Fix
1 Jittery tap swallowed. A tap with 6–20px travel trips cdkDrag's 5px start threshold (isDragging → true) but still emits a tap (within the 20px tapSlop); the old onTileTap isDragging guard then discarded that legitimate tap, so the menu failed to open. Common on a rocking marine display. (adversarial + correctness) P3 Removed the guard. Tap-vs-drag is arbitrated by the gesture directive's movement threshold alone — a real reorder moves past tapSlop and emits no tap. This also fixes the post-drop tap-swallow correctness raised and removes the app-global-signal dependency.
2 editDashboard / duplicateDashboard only called internally after the refactor — should be private (consistency with confirmDelete). (maintainability) P3 Made both private.
3 Test gaps: save/apply path never exercised (dialog mock always emitted null), keyboard onTileKey untested, stale-index guard untested, and 4 tests leaked real CDK overlays by not spying open. (testing ×3, standards) P2 Spec expanded to 10 tests; spy ActionMenuComponent.open in beforeEach + fixture.destroy(); added save-apply, keyboard, and stale-index cases.

Verified sound (adversarial traced 6 break scenarios)

Real reorder never pops the menu (moves past tapSlop → no tap); deleting the last page recreates a blank "Page 1" with activeDashboard reset; double-tap is inert (enableDoubleTap=false); the mat-menu/bottom-drawer breakpoint + position:fixed anchor keep the menu on-screen; delete cannot proceed without confirmation (only the Confirm button closes true). No standards violations (uses unknown not any, serving path untouched, OnPush retained, no stub drift).

Residual risks (safe today, documented — not acted on)

  • _menuIndex is a positional index captured at menu-open and re-read at action time. Safe because both menu variants are modal (backdrop blocks tile interaction) and dashboards() has no live async mutation path. If a future feature adds live sync (multi-tab storage events, remote push, profile switch), switch to capturing dashboard.id. drop() reorder coverage remains a pre-existing gap.

Verdict: ready. lint clean, build green, spec 10/10 locally; CI runs the full matrix.

@mairas mairas merged commit 6842511 into main Jul 4, 2026
7 checks passed
@mairas mairas deleted the feat/dashboards-editor-single-tap branch July 4, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR-4: Align Settings → Dashboards editor to the single-tap popup model

1 participant