From aae94b1da2fface92fbf43abd1a56cc0b1414e4b Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 7 Oct 2026 12:48:49 -0700 Subject: [PATCH] fix(canvas): keep the empty-container block popover inside the viewport The popover is measured once open and clamped to the viewport with an 8px margin; near the bottom edge it flips above the '+'. Refs #41 Co-Authored-By: Claude Opus 5.5 --- craft/src/editor/EmptyContainerAdd.test.tsx | 38 +++++++++++++++++++- craft/src/editor/EmptyContainerAdd.tsx | 40 ++++++++++++++++++++- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/craft/src/editor/EmptyContainerAdd.test.tsx b/craft/src/editor/EmptyContainerAdd.test.tsx index 17cefd0..7c4f11f 100644 --- a/craft/src/editor/EmptyContainerAdd.test.tsx +++ b/craft/src/editor/EmptyContainerAdd.test.tsx @@ -1,7 +1,7 @@ import { describe, test, expect, afterEach } from 'vitest'; import React from 'react'; import { renderEditorHarness, EditorHarness } from '../test-utils/editorHarness'; -import { EmptyContainerAdd } from './EmptyContainerAdd'; +import { EmptyContainerAdd, clampPopoverPosition } from './EmptyContainerAdd'; import { insertBlock } from '../panels/left/blocks-registry'; let h: EditorHarness; @@ -86,3 +86,39 @@ describe('EmptyContainerAdd', () => { expect(plus()).toBeNull(); }); }); + +describe('EmptyContainerAdd popover viewport clamp (#41 follow-up)', () => { + const vp = { width: 1000, height: 800 }; + const size = { width: 240, height: 96 }; + test('centred below the "+" when it fits', () => { + expect(clampPopoverPosition({ x: 500, y: 300 }, size, vp)).toEqual({ left: 380, top: 322 }); + }); + test('clamped at the left and right edges', () => { + expect(clampPopoverPosition({ x: 20, y: 300 }, size, vp).left).toBe(8); + expect(clampPopoverPosition({ x: 990, y: 300 }, size, vp).left).toBe(1000 - 240 - 8); + }); + test('flips above the "+" near the bottom edge', () => { + expect(clampPopoverPosition({ x: 500, y: 760 }, size, vp).top).toBe(760 - 22 - 96); + }); + test('never above the top edge on a tiny viewport', () => { + const p = clampPopoverPosition({ x: 50, y: 40 }, size, { width: 200, height: 100 }); + expect(p.left).toBe(8); + expect(p.top).toBe(8); + }); + test('the rendered popover uses the clamped position (near the right edge)', () => { + seed(); + const { innerId } = (() => { + let secondId = ''; + h.act(() => { secondId = insertBlock(h.query, h.actions, 'section', { parentId: 'ROOT', select: false })!; }); + return { innerId: Object.values(h.query.node(secondId).get().data.linkedNodes as Record)[0] }; + })(); + const dom = h.query.node(innerId).get().dom as HTMLElement; + const right = window.innerWidth - 4; + dom.getBoundingClientRect = () => ({ left: right - 20, top: 10, width: 20, height: 20, right, bottom: 30, x: right - 20, y: 10, toJSON() {} }) as DOMRect; + h.act(() => { h.actions.selectNode(innerId); }); + h.act(() => { plus()!.click(); }); + const pop = document.querySelector('.empty-container-add-pop') as HTMLElement; + expect(parseFloat(pop.style.left)).toBe(window.innerWidth - 240 - 8); + expect(parseFloat(pop.style.left) + 240).toBeLessThanOrEqual(window.innerWidth); + }); +}); diff --git a/craft/src/editor/EmptyContainerAdd.tsx b/craft/src/editor/EmptyContainerAdd.tsx index ca88eb0..188c6ae 100644 --- a/craft/src/editor/EmptyContainerAdd.tsx +++ b/craft/src/editor/EmptyContainerAdd.tsx @@ -6,6 +6,31 @@ import { BLOCKS, insertBlock } from '../panels/left/blocks-registry'; const QUICK_BLOCK_IDS = ['heading', 'text', 'button', 'image', 'columns-2', 'spacer']; const QUICK_LABELS: Record = { 'columns-2': 'Columns' }; +/** Fallback popover size before it has been measured (3x2 grid). */ +const POP_FALLBACK = { width: 240, height: 96 }; +const POP_MARGIN = 8; +const POP_GAP = 22; + +/** + * Where the block popover goes for a "+" centred at `anchor`: centred below + * it, flipped above when it would run off the bottom, and clamped so it stays + * inside the viewport (#41 follow-up). + */ +export function clampPopoverPosition( + anchor: { x: number; y: number }, + size: { width: number; height: number }, + viewport: { width: number; height: number }, +): { left: number; top: number } { + const maxLeft = Math.max(POP_MARGIN, viewport.width - size.width - POP_MARGIN); + const left = Math.min(Math.max(anchor.x - size.width / 2, POP_MARGIN), maxLeft); + let top = anchor.y + POP_GAP; + if (top + size.height > viewport.height - POP_MARGIN) { + const above = anchor.y - POP_GAP - size.height; + top = above >= POP_MARGIN ? above : viewport.height - size.height - POP_MARGIN; + } + return { left, top: Math.max(POP_MARGIN, top) }; +} + /** * Inline "+" for an empty container/section/column: shown for the hovered or * selected canvas node that has zero children (never ROOT). Click opens a @@ -32,6 +57,8 @@ export const EmptyContainerAdd: React.FC = () => { const [keepId, setKeepId] = useState(null); const [open, setOpen] = useState(false); const rootRef = useRef(null); + const popRef = useRef(null); + const [popSize, setPopSize] = useState(POP_FALLBACK); const stillEmpty = (id: string | null): id is string => { if (!id) return false; @@ -79,6 +106,15 @@ export const EmptyContainerAdd: React.FC = () => { }; }, [open, close]); + // Measure the open popover so the clamp uses its real size. + useLayoutEffect(() => { + if (!open || !popRef.current) return; + const r = popRef.current.getBoundingClientRect(); + if (r.width > 0 && r.height > 0 && (r.width !== popSize.width || r.height !== popSize.height)) { + setPopSize({ width: r.width, height: r.height }); + } + }, [open, pos, popSize]); + // Target vanished (filled, dragged, deselected): drop popover state. useEffect(() => { if (!nodeId && open) setOpen(false); }, [nodeId, open]); @@ -89,6 +125,8 @@ export const EmptyContainerAdd: React.FC = () => { close(); }; + const popPos = clampPopoverPosition(pos, popSize, { width: window.innerWidth, height: window.innerHeight }); + const blocks = QUICK_BLOCK_IDS .map((id) => BLOCKS.find((b) => b.id === id)) .filter((b): b is NonNullable => !!b); @@ -108,7 +146,7 @@ export const EmptyContainerAdd: React.FC = () => { {open && ( -
+
{blocks.map((b) => (