From 0ee0422ebf235d0cf244d9e9f52bfb1674a899e2 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Fri, 24 Jul 2026 10:29:56 +0200 Subject: [PATCH 1/6] fix(ew): rework canvas selection-toolbar visibility around an explicit active surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toolbar's visibility was derived from the doc view's `view.hasFocus()` while that same value was faked to keep collab awareness working in WYSIWYG mode — a self-contradiction that made the toolbar flicker, drop out, or fail to appear (especially on the WYSIWYG side of split view). Introduce a single ToolbarController that owns visibility, derived once per frame from an explicit active surface ('doc' | 'wysiwyg') plus selection/mode — never from focus. Editors emit intent; nothing else shows/hides the toolbar. Key fixes: - Null `cursor-move` (a da-nx per-block blur) is awareness-only, no longer hides the toolbar — the dominant drop-out. - Detect focus entering the cross-origin iframe via `window` blur + shadow-piercing active element, since the iframe's own focus events don't fire and da-nx sends no message when a click doesn't change the block selection (e.g. end of a line). - The focus lie is scoped to mirror dispatches (collab awareness only) and never read by the visibility layer. - Coalesce visibility updates to one requestAnimationFrame render. Design and investigation notes in docs/canvas-toolbar-architecture.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- blocks/canvas/canvas.js | 2 + blocks/canvas/editor-utils/editor-utils.js | 27 +- .../canvas/editor-utils/selection-toolbar.js | 47 +-- .../canvas/editor-utils/toolbar-controller.js | 195 ++++++++++++ blocks/canvas/ew-editor-doc/ew-editor-doc.js | 31 +- .../ew-editor-wysiwyg/ew-editor-wysiwyg.js | 19 +- .../ew-editor-wysiwyg/utils/handlers.js | 60 ++-- .../ew-selection-toolbar.js | 35 +-- docs/canvas-toolbar-architecture.md | 277 ++++++++++++++++++ 9 files changed, 590 insertions(+), 103 deletions(-) create mode 100644 blocks/canvas/editor-utils/toolbar-controller.js create mode 100644 docs/canvas-toolbar-architecture.md diff --git a/blocks/canvas/canvas.js b/blocks/canvas/canvas.js index 0e7414c53..3ee948d37 100644 --- a/blocks/canvas/canvas.js +++ b/blocks/canvas/canvas.js @@ -6,6 +6,7 @@ import { persistCanvasEditorView, } from './utils/view.js'; import { shouldAutoOpenAfterPanel } from './utils/panel.js'; +import { toolbarController } from './editor-utils/toolbar-controller.js'; import './ew-canvas-header/ew-canvas-header.js'; import './ew-editor-doc/ew-editor-doc.js'; import './ew-editor-wysiwyg/ew-editor-wysiwyg.js'; @@ -33,6 +34,7 @@ function buildCanvasDocPath(state) { function notifyCanvasEditorActive(mountRoot, view) { const v = normalizeCanvasEditorView(view); + toolbarController.setEditorMode(v); mountRoot.dispatchEvent(new CustomEvent('nx-canvas-editor-active', { bubbles: false, detail: { view: v }, diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 42bf61e5b..117282608 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -2,11 +2,31 @@ import { TextSelection } from 'da-y-wrapper'; import prose2aem from '../../shared/prose2aem.js'; import { getNx } from '../../../scripts/utils.js'; import { daFetch, fetchDaConfigs, getFirstSheet } from '../../shared/utils.js'; -import { getSelectionToolbar } from './selection-toolbar.js'; +import { toolbarController } from './toolbar-controller.js'; import { MESSAGE_TYPES } from '../utils/quick-edit-messages.js'; const { DA_CONTENT } = await import(`${getNx()}/utils/utils.js`); +/** + * Dispatch a mirror transaction while temporarily making `view.hasFocus()` return + * true, then restore it. y-prosemirror's cursor plugin broadcasts this user's + * cursor to collaborators only while the view "has focus"; in WYSIWYG mode the + * iframe owns DOM focus, so without this the local cursor would be cleared on + * every mirrored edit. Scoped to the dispatch so nothing else ever reads the lie — + * toolbar visibility is derived from the active surface, not from focus. + */ +export function dispatchWithFakeFocus(view, tr) { + const hadOwn = Object.hasOwn(view, 'hasFocus'); + const prev = hadOwn ? view.hasFocus : undefined; + view.hasFocus = () => true; + try { + view.dispatch(tr); + } finally { + if (hadOwn) view.hasFocus = prev; + else delete view.hasFocus; + } +} + // --- state.js --- function findInsertedRange(oldText, newText) { @@ -59,11 +79,10 @@ export function updateState(data, ctx) { tr.setSelection(TextSelection.create(tr.doc, restoredFrom, restoredTo)); ctx.suppressRerender = true; - view.dispatch(tr); + dispatchWithFakeFocus(view, tr); ctx.suppressRerender = false; - const tb = getSelectionToolbar(); - if (tb.open && !tb.isInteracting) tb.requestUpdate(); + toolbarController.refresh(); // Sync the updated node (with marks applied) back to the portal's mini editor. // Without this, the portal's editor retains the plain-text version, so the next diff --git a/blocks/canvas/editor-utils/selection-toolbar.js b/blocks/canvas/editor-utils/selection-toolbar.js index 5cf1da548..2c725f136 100644 --- a/blocks/canvas/editor-utils/selection-toolbar.js +++ b/blocks/canvas/editor-utils/selection-toolbar.js @@ -1,5 +1,6 @@ /* eslint-disable import/no-unresolved -- importmap */ import { Plugin, PluginKey, NodeSelection } from 'da-y-wrapper'; +import { toolbarController } from './toolbar-controller.js'; const NON_TEXT_NODES = new Set(['table']); @@ -11,42 +12,31 @@ export const NX_QUICK_EDIT_CLEAR_IFRAME_SELECTION_ORIGIN_META = 'nxClearQuickEdi const selectionToolbarOriginKey = new PluginKey('nxSelectionToolbarOrigin'); -function getSelectionOriginFromIframe(state) { +export function getSelectionOriginFromIframe(state) { return selectionToolbarOriginKey.getState(state)?.fromIframe ?? false; } -let toolbar; -let componentLoaded; - export function getSelectionToolbar() { - if (toolbar) return toolbar; - componentLoaded ??= import('../ew-selection-toolbar/ew-selection-toolbar.js'); - toolbar = document.createElement('ew-selection-toolbar'); - document.body.append(toolbar); - return toolbar; + return toolbarController.ensureToolbar(); } export function setSelectionToolbarCtx({ org = null, site = null, sourceUrl = null } = {}) { - const tb = getSelectionToolbar(); + const tb = toolbarController.ensureToolbar(); tb.org = org; tb.site = site; tb.sourceUrl = sourceUrl; } -export function hideSelectionToolbar() { - toolbar?.hide?.(); -} - export function openLinkDialog(view) { - getSelectionToolbar().openLinkDialog(view); + toolbarController.ensureToolbar().openLinkDialog(view); } export function openAltDialog() { - getSelectionToolbar().openAltDialog(); + toolbarController.ensureToolbar().openAltDialog(); } export function triggerAddImage() { - getSelectionToolbar().triggerAddImage(); + toolbarController.ensureToolbar().triggerAddImage(); } function isNonTextSelection({ selection }) { @@ -54,19 +44,6 @@ function isNonTextSelection({ selection }) { && NON_TEXT_NODES.has(selection.node.type.name); } -function syncToolbar(view) { - if (!view) return; - const tb = getSelectionToolbar(); - if (tb.linkDialogOpen || tb.altDialogOpen || tb.isInteracting) return; - if (isNonTextSelection(view.state)) { - hideSelectionToolbar(); - return; - } - if (!view.hasFocus()) return; - tb.view = view; - tb.show(); -} - export function createSelectionToolbarPlugin() { return new Plugin({ key: selectionToolbarOriginKey, @@ -84,14 +61,14 @@ export function createSelectionToolbarPlugin() { view() { return { update(view) { - const header = document.querySelector('ew-canvas-header'); - const ev = header?.editorView; - if (ev !== 'content' && ev !== 'split') return; + // Iframe-origin dispatches are owned by the wysiwyg handlers; the doc + // plugin only reports the *doc* selection, and never claims the surface + // (activation comes from real focus — see toolbar-controller.js). if (getSelectionOriginFromIframe(view.state)) return; - syncToolbar(view); + toolbarController.setDocSelection({ showable: !isNonTextSelection(view.state) }); }, destroy() { - hideSelectionToolbar(); + toolbarController.reset(); }, }; }, diff --git a/blocks/canvas/editor-utils/toolbar-controller.js b/blocks/canvas/editor-utils/toolbar-controller.js new file mode 100644 index 000000000..24a70f527 --- /dev/null +++ b/blocks/canvas/editor-utils/toolbar-controller.js @@ -0,0 +1,195 @@ +/** + * Single owner of the selection-toolbar's visibility across the canvas doc editor + * and the WYSIWYG iframe. See docs/canvas-toolbar-architecture.md. + * + * Nothing outside this module shows, hides, or positions the toolbar. Callers emit + * intent (activate / deactivate / selection / editor-mode) and this module derives + * visibility once per animation frame from an explicit "active surface" — never + * from `view.hasFocus()`, which lies while the user edits in the iframe. + */ + +let toolbarEl; +let toolbarLoading; +let pointerdownInstalled = false; +let windowBlurInstalled = false; + +const state = { + activeSurface: null, // 'doc' | 'wysiwyg' | null + docView: null, // the single ProseMirror view — always the command target + iframeEl: null, // for outside-click hit-testing + showable: false, // current selection is one the toolbar serves + editorMode: 'layout', // 'layout' | 'content' | 'split' +}; + +function ensureToolbar() { + if (toolbarEl) return toolbarEl; + toolbarEl = document.createElement('ew-selection-toolbar'); + document.body.append(toolbarEl); + // The element definition loads lazily; re-render once it upgrades so a render + // scheduled before the module resolved takes effect. + toolbarLoading ??= import('../ew-selection-toolbar/ew-selection-toolbar.js') + // eslint-disable-next-line no-use-before-define -- mutually recursive with render + .then(() => scheduleRender()); + return toolbarEl; +} + +function editorModeAllows(surface) { + if (surface === 'doc') return state.editorMode === 'content' || state.editorMode === 'split'; + if (surface === 'wysiwyg') return state.editorMode === 'layout' || state.editorMode === 'split'; + return false; +} + +/** The one visibility predicate. Interaction state is pulled from the element so + * a dialog / picker / menu is the single source of truth for "is interacting". */ +function shouldShow(tb) { + if (tb.linkDialogOpen || tb.altDialogOpen || tb.isInteracting) return false; + return state.activeSurface !== null + && state.showable + && editorModeAllows(state.activeSurface); +} + +let renderQueued = false; +function render() { + const tb = ensureToolbar(); + // Element not upgraded yet; ensureToolbar re-renders when its module resolves. + if (typeof tb.show !== 'function') return; + // Only assign a known view; never clobber with null (teardown just hides, and a + // fresh view arrives via setDocView on the next load). + if (state.docView) tb.view = state.docView; + if (shouldShow(tb)) { + tb.show(); + } else if (!tb.linkDialogOpen && !tb.altDialogOpen && !tb.isInteracting) { + tb.hide(); + } +} + +function scheduleRender() { + if (renderQueued) return; + renderQueued = true; + requestAnimationFrame(() => { + renderQueued = false; + render(); + }); +} + +/** Pierce shadow roots to find the truly focused element. `document.activeElement` + * stops at a shadow host, so a focused iframe nested in a shadow root reports the + * host, not the iframe. */ +function deepActiveElement() { + let el = document.activeElement; + while (el?.shadowRoot?.activeElement) el = el.shadowRoot.activeElement; + return el; +} + +/** Detect focus entering the wysiwyg iframe. The iframe element's own focus event + * is unreliable for cross-origin frames, and da-nx sends no message when a click + * doesn't change the block selection (e.g. clicking where the caret already sits). + * When focus moves into an iframe the parent window blurs and the (deep) active + * element becomes that iframe — a robust, message-independent signal. */ +function installIframeFocusDetection() { + if (windowBlurInstalled) return; + windowBlurInstalled = true; + window.addEventListener('blur', () => { + setTimeout(() => { + if (state.iframeEl && deepActiveElement() === state.iframeEl) { + // Entering an editable pane; assume showable so the toolbar appears even + // when no positional message follows. A later node-select (e.g. a table) + // refines it. + state.activeSurface = 'wysiwyg'; + state.showable = true; + scheduleRender(); + } + }, 0); + }); +} + +function installOutsidePointerdown() { + if (pointerdownInstalled) return; + pointerdownInstalled = true; + document.addEventListener('pointerdown', (e) => { + if (state.activeSurface === null) return; + const path = e.composedPath(); + if (toolbarEl && path.includes(toolbarEl)) return; + if (state.docView?.dom && path.includes(state.docView.dom)) return; + if (state.iframeEl && path.includes(state.iframeEl)) return; + // A real pointerdown in the parent document outside every editing surface — + // the user is leaving. (Clicks inside the cross-origin iframe never reach here, + // and are handled by the iframe's own blur.) + state.activeSurface = null; + scheduleRender(); + }); +} + +export const toolbarController = { + ensureToolbar, + + /** Register the doc editor's view (the command target). */ + setDocView(view) { + state.docView = view ?? null; + installOutsidePointerdown(); + scheduleRender(); + }, + + setIframe(iframeEl) { + state.iframeEl = iframeEl ?? null; + if (iframeEl) installIframeFocusDetection(); + }, + + setEditorMode(mode) { + if (state.editorMode === mode) return; + state.editorMode = mode; + scheduleRender(); + }, + + /** The user is now editing in `surface` (driven by real focus / positional intent). */ + activate(surface, { iframeEl } = {}) { + if (surface !== 'doc' && surface !== 'wysiwyg') return; + if (iframeEl !== undefined) state.iframeEl = iframeEl; + state.activeSurface = surface; + installOutsidePointerdown(); + scheduleRender(); + }, + + /** Ownership-guarded: only the surface that currently owns the toolbar may + * deactivate it, so a late blur from one editor can't wipe the other. */ + deactivate(surface) { + if (surface && state.activeSurface !== surface) return; + state.activeSurface = null; + scheduleRender(); + }, + + /** Doc selection changed. Never claims the surface — a background/collab/mirror + * dispatch must not show the toolbar on a doc the user isn't editing. */ + setDocSelection({ showable }) { + if (state.activeSurface === 'wysiwyg') return; + state.showable = showable; + scheduleRender(); + }, + + /** A positional message from the iframe: the user is editing there. */ + setWysiwygSelection({ showable }) { + state.activeSurface = 'wysiwyg'; + state.showable = showable; + installOutsidePointerdown(); + scheduleRender(); + }, + + /** Re-query the toolbar's command/visibility state (e.g. after a command runs + * or a dialog/picker closes) without changing surface. */ + refresh() { + scheduleRender(); + }, + + /** After a toolbar command / dialog close — return focus to the active surface. */ + restoreFocus() { + if (state.activeSurface === 'doc') state.docView?.focus(); + // wysiwyg: focus never left the iframe (toolbar suppresses it via mousedown). + }, + + reset() { + state.activeSurface = null; + state.showable = false; + state.docView = null; + scheduleRender(); + }, +}; diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index 7eed6e97f..c9ed69d19 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -25,7 +25,8 @@ import { createTrackingPlugin } from '../editor-utils/prose-diff.js'; import { resolveEditorDocSession } from './utils/load-editor-doc.js'; import { afterNextPaint, ensureProseMountedInShadow } from './utils/shadow-mount.js'; import { teardownEditorDocResources } from './utils/teardown.js'; -import { hideSelectionToolbar, setSelectionToolbarCtx } from '../editor-utils/selection-toolbar.js'; +import { setSelectionToolbarCtx } from '../editor-utils/selection-toolbar.js'; +import { toolbarController } from '../editor-utils/toolbar-controller.js'; import { createExtensionsBridgePlugin } from '../editor-utils/extensions-bridge.js'; import { MESSAGE_TYPES } from '../utils/quick-edit-messages.js'; @@ -47,6 +48,7 @@ export class EwEditorDoc extends LitElement { this.quickEditPort = undefined; this._teardown(); setSelectionToolbarCtx(); + toolbarController.reset(); this._error = undefined; this._lastDocBlockIndex = undefined; this._lastDocSelKey = undefined; @@ -178,6 +180,28 @@ export class EwEditorDoc extends LitElement { wireQuickEditControllerPort(this._controllerCtx); } + _wireProseFocus(view, proseEl) { + this._unwireProseFocus?.(); + const onFocusIn = () => toolbarController.activate('doc'); + const onFocusOut = () => { + // Defer so focus can settle. If it landed on the toolbar (button/dialog), + // stay active; otherwise the user left the doc surface. + setTimeout(() => { + const tb = toolbarController.ensureToolbar(); + const active = document.activeElement; + if (active && (active === tb || tb.contains(active))) return; + toolbarController.deactivate('doc'); + }, 0); + }; + proseEl.addEventListener('focusin', onFocusIn); + proseEl.addEventListener('focusout', onFocusOut); + this._unwireProseFocus = () => { + proseEl.removeEventListener('focusin', onFocusIn); + proseEl.removeEventListener('focusout', onFocusOut); + this._unwireProseFocus = undefined; + }; + } + _setupAwareness(wsProvider) { if (this._awarenessOff) { this._awarenessOff(); @@ -198,6 +222,7 @@ export class EwEditorDoc extends LitElement { _teardown() { this._stopObservingUndoManager(); + this._unwireProseFocus?.(); const { wsProvider, view, proseEl } = this._proseContext ?? {}; teardownEditorDocResources({ clearPortHandler: () => this._clearControllerPort(), @@ -263,6 +288,8 @@ export class EwEditorDoc extends LitElement { this._proseContext = { proseEl, wsProvider, view, ydoc, undoManager }; setSelectionToolbarCtx({ org: this.ctx?.org, site: this.ctx?.repo, sourceUrl }); + toolbarController.setDocView(view); + this._wireProseFocus(view, proseEl); this._setupAwareness(wsProvider); this._observeUndoManager(undoManager); this._emitHtmlChange(); @@ -283,7 +310,6 @@ export class EwEditorDoc extends LitElement { this._onCanvasEditorActive = (e) => { const view = e.detail?.view; this.hidden = view === 'layout'; - hideSelectionToolbar(); }; this.parentElement?.addEventListener('nx-canvas-editor-active', this._onCanvasEditorActive); this._onWysiwygPortReady = (e) => { @@ -315,6 +341,7 @@ export class EwEditorDoc extends LitElement { this._unsubscribeSelect?.(); this._teardown(); setSelectionToolbarCtx(); + toolbarController.reset(); super.disconnectedCallback(); } diff --git a/blocks/canvas/ew-editor-wysiwyg/ew-editor-wysiwyg.js b/blocks/canvas/ew-editor-wysiwyg/ew-editor-wysiwyg.js index 8a7b2964f..4d28253d7 100644 --- a/blocks/canvas/ew-editor-wysiwyg/ew-editor-wysiwyg.js +++ b/blocks/canvas/ew-editor-wysiwyg/ew-editor-wysiwyg.js @@ -2,7 +2,7 @@ import { LitElement, html } from 'da-lit'; import { getNx } from '../../../scripts/utils.js'; import { getPreviewOrigin, fetchWysiwygCookie, fetchWysiwygBranch } from '../editor-utils/editor-utils.js'; import { initIms as loadIms } from '../../shared/utils.js'; -import { hideSelectionToolbar } from '../editor-utils/selection-toolbar.js'; +import { toolbarController } from '../editor-utils/toolbar-controller.js'; import { MESSAGE_TYPES } from '../utils/quick-edit-messages.js'; const { loadStyle } = await import(`${getNx()}/utils/utils.js`); @@ -100,7 +100,6 @@ export class EwEditorWysiwyg extends LitElement { const view = this._canvasActiveView ?? 'layout'; const showWysiwyg = view === 'layout' || view === 'split'; this.hidden = !showWysiwyg; - hideSelectionToolbar(); } _resetCookieStateForCtxChange() { @@ -189,6 +188,7 @@ export class EwEditorWysiwyg extends LitElement { const { org, repo, path } = this.ctx ?? {}; if (!iframe?.contentWindow || !org || !repo || !path) return; + toolbarController.setIframe(iframe); this.removeAttribute(WYSIWYG_PORT_READY_ATTR); this._clearQuickEditRetry(); this._syncCanvasVisibility(); @@ -205,8 +205,20 @@ export class EwEditorWysiwyg extends LitElement { this._scheduleQuickEditInitRetries(send); } + _onIframeFocus() { + const iframe = this.shadowRoot?.querySelector('iframe'); + toolbarController.activate('wysiwyg', { iframeEl: iframe }); + } + _onIframeBlur() { - hideSelectionToolbar(); + // Defer so focus can settle. If it landed on the toolbar (button/dialog), keep + // the wysiwyg surface active; otherwise the user has left the pane. + setTimeout(() => { + const tb = toolbarController.ensureToolbar(); + const active = document.activeElement; + if (active && (active === tb || tb.contains(active))) return; + toolbarController.deactivate('wysiwyg'); + }, 0); } render() { @@ -228,6 +240,7 @@ export class EwEditorWysiwyg extends LitElement { allow="local-network-access" class="ew-editor-wysiwyg-iframe" @load=${this._onIframeLoad} + @focus=${this._onIframeFocus} @blur=${this._onIframeBlur} > `; diff --git a/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js b/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js index da180e688..bc6187c67 100644 --- a/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js +++ b/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js @@ -1,10 +1,10 @@ import { TextSelection, NodeSelection, yUndo, yRedo } from 'da-y-wrapper'; import { - getSelectionToolbar, NX_QUICK_EDIT_IFRAME_SELECTION_META, NX_QUICK_EDIT_CLEAR_IFRAME_SELECTION_ORIGIN_META, } from '../../editor-utils/selection-toolbar.js'; -import { editorSelectChange } from '../../editor-utils/editor-utils.js'; +import { editorSelectChange, dispatchWithFakeFocus } from '../../editor-utils/editor-utils.js'; +import { toolbarController } from '../../editor-utils/toolbar-controller.js'; import { getActiveBlockIndex } from '../../editor-utils/blocks.js'; export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { @@ -12,10 +12,10 @@ export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { if (!view || !wsProvider) return; if (cursorOffset == null || textCursorOffset == null) { - delete view.hasFocus; + // Per-block blur from the iframe — its documented purpose is clearing the + // remote cursor, NOT hiding the toolbar. The user is still in the pane while + // the iframe holds focus; toolbar deactivation comes from the iframe's blur. wsProvider.awareness.setLocalStateField('cursor', null); - const tb = getSelectionToolbar(); - if (!tb.isInteracting && !tb.linkDialogOpen) tb.hide?.(); return; } @@ -29,8 +29,6 @@ export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { return; } - view.hasFocus = () => true; - const { tr } = state; tr.setSelection(TextSelection.create(state.doc, position)); @@ -58,13 +56,9 @@ export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { } ctx.suppressRerender = true; - view.dispatch(tr.scrollIntoView()); + dispatchWithFakeFocus(view, tr.scrollIntoView()); ctx.suppressRerender = false; - const tb = getSelectionToolbar(); - if (!tb.linkDialogOpen && !tb.isInteracting) { - tb.view = view; - tb.show(); - } + toolbarController.setWysiwygSelection({ showable: true }); const blockIndex = getActiveBlockIndex(view); if (blockIndex !== ctx.lastBlockIndex) { ctx.lastBlockIndex = blockIndex; @@ -80,19 +74,11 @@ export function handleUndoRedo(data, ctx) { const { action } = data; const view = ctx?.view; if (!view) return; - // hasFocus may be overridden to () => true by the cursor-move hack; temporarily - // restore the prototype so ProseMirror skips _isDomSelectionInView during the - // undo dispatch (the editor may be in a hidden or unfocused state). - const hadHasFocus = Object.hasOwn(view, 'hasFocus'); - delete view.hasFocus; - if (action === 'undo') { yUndo(view.state); } else if (action === 'redo') { yRedo(view.state); } - - if (hadHasFocus) view.hasFocus = () => true; } export function handleNewVersion() { @@ -114,10 +100,9 @@ export function handleStoredMarks({ marks }, ctx) { const { tr } = state; tr.setStoredMarks(parsedMarks); ctx.suppressRerender = true; - view.dispatch(tr); + dispatchWithFakeFocus(view, tr); ctx.suppressRerender = false; - const tb = getSelectionToolbar(); - if (tb.open && !tb.isInteracting) tb.requestUpdate(); + toolbarController.refresh(); } catch (e) { // eslint-disable-next-line no-console console.error('[quick-edit-controller] handleStoredMarks failed', e?.message); @@ -135,7 +120,7 @@ export function handleSelectionChange({ anchor, head }, ctx, { fromQuickEditIfra tr.setSelection(TextSelection.create(state.doc, a, h)); if (fromQuickEditIframe) tr.setMeta(NX_QUICK_EDIT_IFRAME_SELECTION_META, true); ctx.suppressRerender = true; - view.dispatch(tr); + dispatchWithFakeFocus(view, tr); ctx.suppressRerender = false; return true; } catch (e) { @@ -145,34 +130,29 @@ export function handleSelectionChange({ anchor, head }, ctx, { fromQuickEditIfra } } -function showToolbarInIFrame(ctx) { - const { view } = ctx; - const tb = getSelectionToolbar(); - tb.view = view; - tb.show(); -} - /** PostMessage `selection-change` from wysiwyg iframe: sync PM selection and toolbar. */ export function handleIframeSelectionChange(data, ctx) { const { anchor, head } = data; + const { view } = ctx; if (anchor === head) { - const tb = getSelectionToolbar(); - if (tb.isInteracting) return; - const { view } = ctx; + // Collapsed to a caret: clear the iframe-origin flag so subsequent doc + // transactions are read normally. Still a caret in the iframe, so the toolbar + // stays active/showable. if (view) { const tr = view.state.tr .setMeta(NX_QUICK_EDIT_CLEAR_IFRAME_SELECTION_ORIGIN_META, true) .setMeta('addToHistory', false); ctx.suppressRerender = true; - view.dispatch(tr); + dispatchWithFakeFocus(view, tr); ctx.suppressRerender = false; } + toolbarController.setWysiwygSelection({ showable: true }); return; } if (!handleSelectionChange(data, ctx, { fromQuickEditIframe: true })) return; - showToolbarInIFrame(ctx); + toolbarController.setWysiwygSelection({ showable: true }); } function srcFileName(src) { @@ -234,7 +214,7 @@ export function handleNodeSelect({ node }, ctx) { .setSelection(TextSelection.near(state.doc.resolve(state.selection.from), 1)) .setMeta('addToHistory', false); ctx.suppressRerender = true; - view.dispatch(tr); + dispatchWithFakeFocus(view, tr); ctx.suppressRerender = false; return; } @@ -245,8 +225,10 @@ export function handleNodeSelect({ node }, ctx) { .scrollIntoView() .setMeta('addToHistory', false); ctx.suppressRerender = true; - view.dispatch(tr); + dispatchWithFakeFocus(view, tr); ctx.suppressRerender = false; + // Tables have their own UI; the toolbar hides for them. Images keep it. + toolbarController.setWysiwygSelection({ showable: node.anchorType !== 'table' }); } catch (e) { // eslint-disable-next-line no-console console.error('[quick-edit-controller] handleNodeSelect failed', e?.message); diff --git a/blocks/canvas/ew-selection-toolbar/ew-selection-toolbar.js b/blocks/canvas/ew-selection-toolbar/ew-selection-toolbar.js index 7cc14f7f1..2e051dbee 100644 --- a/blocks/canvas/ew-selection-toolbar/ew-selection-toolbar.js +++ b/blocks/canvas/ew-selection-toolbar/ew-selection-toolbar.js @@ -6,6 +6,7 @@ import { getLinkInfoInSelection, applyLink, } from '../editor-utils/command-helpers.js'; +import { toolbarController } from '../editor-utils/toolbar-controller.js'; const { loadStyle } = await import(`${getNx()}/utils/utils.js`); @@ -54,20 +55,6 @@ class EwSelectionToolbar extends LitElement { connectedCallback() { super.connectedCallback(); this.shadowRoot.adoptedStyleSheets = [styles]; - this._onOutsidePointerDown = (e) => { - if (!this.open) return; - const path = e.composedPath(); - if (path.includes(this)) return; - const editorDom = this.view?.dom; - if (editorDom && path.includes(editorDom)) return; - this.hide(); - }; - document.addEventListener('pointerdown', this._onOutsidePointerDown); - } - - disconnectedCallback() { - super.disconnectedCallback(); - document.removeEventListener('pointerdown', this._onOutsidePointerDown); } get _picker() { return this.shadowRoot?.querySelector('nx-picker'); } @@ -124,7 +111,8 @@ class EwSelectionToolbar extends LitElement { if (cmd) { cmd.apply(this.view); this.requestUpdate(); - this.view.focus(); + toolbarController.restoreFocus(); + toolbarController.refresh(); } } @@ -162,7 +150,10 @@ class EwSelectionToolbar extends LitElement { if (!id) return; COMMAND_BY_ID.get(id)?.apply(this.view); this.requestUpdate(); - if (!this._linkDialogOpen && !this._altDialogOpen) this.view.focus(); + if (!this._linkDialogOpen && !this._altDialogOpen) { + toolbarController.restoreFocus(); + toolbarController.refresh(); + } } /* ---- Link dialog ---- */ @@ -185,14 +176,16 @@ class EwSelectionToolbar extends LitElement { _closeLinkDialog() { this._linkDialogOpen = false; - this.view?.focus(); + toolbarController.restoreFocus(); + toolbarController.refresh(); } _onLinkDialogSubmit(e) { const { href, text } = e.detail; this._closeLinkDialog(); applyLink(this.view, { href, text }); - this.view.focus(); + toolbarController.restoreFocus(); + toolbarController.refresh(); } get linkDialogOpen() { return this._linkDialogOpen ?? false; } @@ -208,7 +201,8 @@ class EwSelectionToolbar extends LitElement { _closeAltDialog() { this._altDialogOpen = false; - this.view?.focus(); + toolbarController.restoreFocus(); + toolbarController.refresh(); } _onAltDialogSubmit(e) { @@ -217,7 +211,8 @@ class EwSelectionToolbar extends LitElement { const { pos } = this.view.state.selection.$anchor; this._closeAltDialog(); this.view.dispatch(this.view.state.tr.setNodeAttribute(pos, 'alt', alt)); - this.view.focus(); + toolbarController.restoreFocus(); + toolbarController.refresh(); } get altDialogOpen() { return this._altDialogOpen ?? false; } diff --git a/docs/canvas-toolbar-architecture.md b/docs/canvas-toolbar-architecture.md new file mode 100644 index 000000000..725888702 --- /dev/null +++ b/docs/canvas-toolbar-architecture.md @@ -0,0 +1,277 @@ +# Canvas selection-toolbar architecture + +Status: design (pre-implementation) +Scope: the shared selection toolbar across the canvas doc editor and the WYSIWYG +iframe, in all three view modes (`layout`, `content`, `split`). + +This document is the **authority contract** for a redesign. It exists because the +current implementation, and a prior refactor attempt (PR #1018, closed), both +suffer from the toolbar intermittently failing to appear — most visibly on the +WYSIWYG (layout) side of split view. The contract below is grounded in three +investigation spikes whose findings are summarised at the end. + +--- + +## 1. Background: what exists today + +- There is **one** ProseMirror view — the doc editor's (`ew-editor-doc`). It is + always the command target. The WYSIWYG pane is a cross-origin iframe (the da-nx + "quick-edit" preview) running its own editors that mirror state to the doc view + over a `MessageChannel`. Keeping the doc view as the single command target is + correct and is **retained** by this design. +- The toolbar is a single global `` (`position: fixed`) + appended to `document.body`. +- Toolbar visibility is currently written from ~6 places by **two competing + drivers**: + - the ProseMirror plugin in `editor-utils/selection-toolbar.js`, which gates on + `view.hasFocus()`; and + - the iframe message handlers in `ew-editor-wysiwyg/utils/handlers.js`, which + fake `view.hasFocus = () => true` and call `show()/hide()` directly. + +The core defect: visibility is derived from `view.hasFocus()` **while that same +value is being faked**. The two ideas are mutually contradictory, so every fix +on top of the current model spawns a new race. + +--- + +## 2. Root causes (evidence-ranked) + +1. **Null `cursor-move` misused as "hide toolbar" (dominant).** da-nx sends a + `cursor-move` with no offsets as a **per-block blur** signal (v1 debounced + 150ms; v2 immediate). Its documented meaning is *"clear the remote cursor"* + (`nx/utils/message-types.js`). da-live's `handleCursorMove` overloaded it to + also hide the toolbar. Because it is per-block, ordinary actions inside the + iframe (clicking between blocks, clicking a non-editable region, a block + re-rendering after an edit) fire it even though focus never left the iframe — + hiding the toolbar until the next real cursor move. +2. **No coalescing.** A single range drag emits ~30 `selection-change` messages, + each triggering 1–2 visibility recomputes. +3. **The `view.hasFocus()` lie leaks into visibility.** The fake focus is required + for collaboration (see §6) but must not participate in the toolbar decision. + In PR #1018 it still did — a mirror dispatch that ran the visibility plugin + under faked focus could claim the wrong active surface. +4. **`document.activeElement` heuristics across the iframe boundary** are + timing- and browser-dependent (used by #1018's deferred blur handlers). + +Non-issue: a suspected "caret `selection-change` before `cursor-move`" ordering +race on first click **did not reproduce** in the current da-nx build. + +--- + +## 3. Key insight that drives the design + +**"Active surface = WYSIWYG" ⟺ focus is inside the iframe.** While the user clicks +between blocks *inside* the iframe, focus stays in the frame — only da-nx's +internal per-block editors change focus, which the parent never sees. The null +`cursor-move` (a per-block blur) does **not** mean focus left the frame, which is +exactly why it must not drive visibility. + +**How to detect it (corrected after testing).** The obvious signal — a `focus`/ +`blur` event on the `