Skip to content

Preview Inspect Mode (draft for internal review) - #188

Draft
erikfrerejean wants to merge 61 commits into
6from
feature/preview-grid-highlight
Draft

Preview Inspect Mode (draft for internal review)#188
erikfrerejean wants to merge 61 commits into
6from
feature/preview-grid-highlight

Conversation

@erikfrerejean

Copy link
Copy Markdown
Member

Summary

Adds an opt-in Inspect Mode that bridges the CMS grid editor and the rendered preview iframe, in both directions:

  • Editor → Preview: hover any block in the editor tree → matching element highlights in the preview (ancestor tints + floating breadcrumb, smooth scroll into view, 200 ms debounce).
  • Preview → Editor: hover any grid element in the preview → matching block halos in the editor tree. When the element sits inside a collapsed ancestor, the halo falls back to the nearest visible ancestor (dashed outline) and a floating breadcrumb near the halo shows the full Section › Row › Column › Element path with collapsed segments dimmed — so admins never lose orientation on large grids.

The toggle lives in the editor header next to the viewport switcher, persists across reloads via localStorage (grid:inspect-mode), and is a user preference (not per-page).

Status: draft — opened so the work is saved while we internally debate whether to ship.

Architecture at a glance

Three loosely-coupled pieces talking over postMessage:

  1. Markup contract (templates/): every grid element's rendered DOM carries data-grid-element-id="\$ID" + data-grid-element-title="\$Title.ATT". For content elements that don't have their own holder template, GridElement_holder.ss supplies a plain <div> wrapper carrying the attributes.
  2. Preview bundle (client/src/preview/): vanilla-TS inspector loaded only on CMS-preview renders via a new PreviewInspectorExtension. Single delegated mousemove handler, hand-rolled message validators, no Zod (< 4 KB gz).
  3. Editor module (client/src/inspect/): React context + InspectBridgeHost (handshake, debounce, message wiring), InspectHoverDelegate (document-level delegated mousemove so nested bindings compose correctly), InspectOverlay (halo + breadcrumb rendering), InspectToggle (the button).

Full spec & plan live in docs/superpowers/ (untracked per repo convention).

Notable engineering decisions

  • Delegated mousemove over per-element handlers. React's synthetic onMouseEnter doesn't refire when the cursor re-enters a wrapper from one of its descendants — breaking column hover when moving from a child content element back to the column header. Swapped for a single document-level mousemove listener that resolves the innermost [data-node-id] via closest() on every move, matching the preview side's hit-test model.
  • Handshake race fix. Preview bundle echoes grid-inspect:ready on every inbound control message, so the editor's first activate/deactivate always elicits a fresh ready on its now-live listener — closes the window where preview's boot-time ready fires before the editor's useEffect subscribe attaches.
  • Rect-based visibility. SectionBlock keeps descendants mounted when collapsed (CSS-only hide), so querySelector alone falsely claims they're visible. findVisibleById uses getBoundingClientRect() as the source of truth, correctly distinguishing collapsed vs expanded.
  • Content-element wrapper is a plain div. Initially used display: contents for layout invisibility but the inspector's paint depends on a non-zero rect, so content elements became invisible to the halo. Dropped.
  • Preview iframe late-mount. InspectBridgeHost installs a MutationObserver on document.body to re-wire the load listener whenever the preview iframe appears/disappears (the CMS mounts it lazily in some flows).

Test coverage

  • Unit/integration (Vitest + PHPUnit): 847 tests across 76 files, including ancestry walks, message bridge origin/schema filtering, hover binding delegation, overlay halo + breadcrumb rendering (direct/indirect/missing variants), PreviewInspectorExtension on both preview and public requests, template markup contract.
  • E2E (Playwright, 3 specs / 2 journeys):
    • Journey A — admin orients from the preview, hovers preview elements (including inside a collapsed Section), verifies editor halo + breadcrumb, reloads to confirm toggle persistence, hovers again to verify handshake recovery, disables inspect to verify both sides clear.
    • Journey B — admin hovers editor blocks (section, column header, child-card → parent column), verifies preview highlights, tests 200 ms debounce with rapid A/B/A/B, confirms drag suppresses inspect hover, reloads to confirm inspect state + preview handshake survive.

Known open discussion

This PR is intentionally draft — the team is internally debating whether to ship. Questions still on the table:

  • Is the breadcrumb too much visual noise in the editor, or the right amount for large grids?
  • Should the dashed --indirect halo also offer an "Expand to reveal" affordance?
  • Bundle-size impact on the editor side (post-feature bundle.js is 195.90 kB / 56.21 kB gz, up ~15 kB gz from pre-feature baseline, mostly Zod + inspect module).

Test plan

  • Load a page in CMS split mode, click Inspect in the editor header.
  • Hover any block in the editor tree — matching preview element should highlight with blue outline + ancestor tints + floating breadcrumb.
  • Hover any grid element in the preview iframe — matching editor block should halo and scroll into view.
  • Collapse a Section in the editor, then hover one of its descendants in the preview — halo should land on the Section with a dashed outline and the breadcrumb should dim the hidden segments.
  • Reload the edit form — toggle stays on, hover still works.
  • Start a column drag — inspect hover is suppressed during the drag.
  • Toggle off — halo + preview target class clear; normal preview interactions (link clicks, scroll) work again.
  • Verify preview.js + preview.css do NOT load on public (non-CMS) page renders.

The holder template lives at templates/WeDevelop/Grid/Model/GridElement_holder.ss,
not under an Includes/ subdirectory. Passing 'type' => 'Includes' triggers a
MissingTemplateException, so drop it and let SSViewer resolve the class-style
path directly.
`InspectContext.test.tsx` and `InspectBridgeHost.test.tsx` each
redefined the same Map-backed `createMockLocalStorage` /
`installMockLocalStorage` pair. Drift between the two copies would
be a silent correctness bug (tests pass locally, disagree on jsdom
behaviour). Pull the helpers into `client/src/testing/mockLocalStorage.ts`
so both test files import from the same source.
Preview Inspect Mode highlights elements by attaching the
.grid-inspect-target class (outline + background) to the
[data-grid-element-id] wrapper and by reading its
getBoundingClientRect(). A display:contents element has no
principal box, so both CSS painting and the rect read produce
nothing -- container elements (Section/Row/Column) were visible
because their holders attach the attribute to their own real
<section>/<div>, but content elements, which fall through to
GridElement_holder.ss, were invisible to the inspector.

Removing style="display:contents" turns the wrapper into a plain
block-level <div>. Column holders already wrap child output in a
<div>, so the extra box does not change flex/grid participation.

Adds three integration tests to GridHolderMarkupTest:
- testContentElementForTemplateEmitsDataGridElementId confirms
  forTemplate() resolves to GridElement_holder for ContentElement.
- testContentElementHolderIsNotDisplayContents is the regression
  guard for the fix.
- testColumnLoopRendersContentElementWithWrapper exercises the
  realistic Column -> $Elements -> $Me -> forTemplate() flow.
- useCallback on hover handlers was cargo-culted; they're spread onto
  native DOM elements where React uses the latest reference directly.
- ElementCard's {...hoverBinding} now precedes onClick so an explicit
  event handler on the anchor always wins over the spread default.
  (Today hoverBinding only carries optional mouse handlers, but the
  ordering convention matches the other three block components and
  avoids a silent override trap for future readers.)
When the editor re-mounts after a page reload with `enabled=true`
hydrated from localStorage, the preview iframe may have already booted
and emitted its startup `ready` before the parent's subscribe listener
attached. That lost `ready` left `readyRef=false`, the queued activate
never flushed, and inspect stayed inactive until the user manually
toggled off/on.

The existing echo-on-inbound fix only helps when the parent sends
something outbound to trigger an echo — but the post-reload path sent
nothing when queuing. Now we also send a `deactivate` probe in the
queuing branch. The preview echoes `ready`, `readyRef` flips true, and
the queued activate flushes. A no-op deactivate against an already-
inactive inspector has no side effect.
…ransitions

React's synthetic onMouseEnter on a wrapper does not refire when the
cursor re-enters the wrapper from a descendant, so after hovering a
column's ElementCard child and moving back up to the column's own
header the column never re-entered hover state — the preview stayed
highlighted on the child. Replace the per-element onMouseEnter/Leave
with a single document-level mousemove listener that resolves the
innermost [data-node-id] ancestor via closest() and drives
setEditorHover from that — matching how the preview-side inspector
already hit-tests.

- InspectHoverDelegate: new component, one delegated listener,
  drag-suppression via ref, mouseleave on documentElement to clear.
- useInspectHoverBinding: stripped to just data-node-id.
- GridEditor: mount <InspectHoverDelegate /> inside DragContext in both
  editable and readonly branches.
- E2E: new Journey B step exercises the child→parent regression.
- Unit: 10-case InspectHoverDelegate spec covers hit-test depth,
  drag suppression, mouseleave clear, dedupe, and disabled short-circuit.
…apsed container

When a preview-side hover resolves to an element whose editor-tree
ancestors are collapsed, the halo previously just landed on the
nearest visible ancestor (usually the Section header) with no
indication of the full path — the admin couldn't tell where in the
tree the element actually lived without hunting through the collapse
state.

This adds two coordinated signals:

1. Editor-side floating breadcrumb showing the complete
   Section › Row › Column › Element trail, rendered near the halo
   (mirroring the preview-side breadcrumb for symmetry). Segments
   whose containers are currently collapsed are dimmed + italicized
   + carry a subtle corner glyph, so the admin can see at a glance
   which ancestors they'd need to expand to reach the element.

2. Indirect halo variant (dashed, softer colour) when the halo lands
   on a proxy ancestor rather than the real target — distinguishes
   'this is the element' from 'the element is hidden inside this'.

Implementation:
- ancestry.ts: new resolveAncestorPath returning full PathSegment[]
  (id + title + type) so consumers don't look nodes back up in the
  tree on every render.
- InspectContext: preview hover state carries the path.
- InspectBridgeHost: resolves and passes path alongside ancestorIds.
- InspectOverlay: rewritten to render halo + breadcrumb together;
  introduces rect-based visibility check (findVisibleById) so
  CSS-collapsed descendants count as hidden, not just absent ones.
- inspect.scss: breadcrumb surface + --indirect halo variant.

Tests:
- Unit: 3 new ancestry tests for resolveAncestorPath; 4 new
  InspectOverlay breadcrumb tests (full trail render, dimmed hidden
  segments, target marker, empty-path defensive null).
- E2E: Journey A gains a step that collapses Hero section, hovers
  the paragraph in the preview, and asserts the halo becomes
  --indirect while the breadcrumb flags Row/Column/Element as
  data-segment-hidden.
const { result } = renderHook(() => useInspect(), { wrapper });
localStorage.setItem(STORAGE_KEY, 'true');
act(() => {
window.dispatchEvent(new StorageEvent('storage', { key: STORAGE_KEY }));
const { result } = renderHook(() => useInspect(), { wrapper });
localStorage.setItem(STORAGE_KEY, 'true');
act(() => {
window.dispatchEvent(new StorageEvent('storage', { key: 'something-else' }));
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.

1 participant