Imgswap - #1377
Open
hannessolo wants to merge 27 commits into
Open
Imgswap#1377hannessolo wants to merge 27 commits into
hannessolo wants to merge 27 commits into
Conversation
…t active surface
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) <noreply@anthropic.com>
y-prosemirror's cursor plugin broadcasts this user's caret to collaborators only while the doc view "has focus", and clears it on the next update once focus is lost. Its updateCursorInfo is the plugin's view.update callback, so it runs after every transaction. A focus lie scoped to a single mirror dispatch therefore only survived one tick: the next unrelated update (a remote edit, the iframe streaming into Yjs, or the redraw our own setLocalStateField triggers) ran with the real hasFocus() === false and wiped the just-broadcast cursor. The caret flashed to peers and vanished. Gate the lie on the active surface instead: while activeSurface === 'wysiwyg' the doc view reports focus, so the cursor keeps broadcasting for the whole time the iframe owns editing (layout and split). When the user leaves, the real check returns and the cursor is correctly cleared. The lie makes selectionToDOM treat the doc view as owning the selection, but that only writes a DOM selection range (no focus move). The one thing that would steal focus is view.focus(), so neuter it while the iframe is active — no caller (drop handler, command, restoreFocus) can pull focus off the iframe and revive the toolbar-visibility bugs. dispatchWithFakeFocus is retained as a complement for the instant before the surface flips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The WYSIWYG iframe owns block-level structure, so the shared selection toolbar now renders a surface-appropriate button set. The controller pushes the active surface onto the element, and the element gates its sections: - doc surface: full toolbar (block-type picker, marks, structure, tables, links, images incl. add-image) - wysiwyg surface: inline marks, link controls, and image alt-text editing only — the block-type picker, list/structure, table controls, and the add-image action are dropped (block insertion and block structure belong to the iframe) In split view the button set follows the pane you're editing. Ports the per-surface intent from the earlier PR #1018 into the current architecture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handleCursorMove preserved state.storedMarks whenever the new position had no adjacent marks, so moving the caret off bold text left bold queued on unmarked text. That preservation was only meant to survive the same-position cursor-move the iframe re-reports right after a toolbar toggle — not a real move. Track the last cursor offset and branch on it: a genuine move resets stored marks to what's at the new location (nothing on unmarked text), while a same-position re-report keeps a toolbar-toggled mark until the user types or actually moves. Also forget the position on iframe blur so re-entry counts as a move. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getSelectionToolbar() was a one-line pass-through to toolbarController.ensureToolbar(); production code already calls the controller directly. Remove it and point the remaining test at ensureToolbar(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion Fork daCursorPlugin so the collab cursor no longer relies on a faked view.hasFocus(), which caused the doc view to steal focus and the caret from the WYSIWYG surface in split view. Toolbar visibility is now owned solely by toolbar-controller via setSurface() with per-surface slots. Also fixes latent regressions from the unfinished canvas-bus migration: CHAT_EVENT import path, dead nx-canvas-editor-active and nx-wysiwyg-port-ready DOM listeners, and the two missing canWrite flags that left the quick-edit iframe read-only and silently dropped edits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolves the canvas conflicts in favour of main's feature set (comments, block-edit modal, table menu, mediaBusImage, proseIndex navigation, preflight) with this branch's toolbar/focus refactor re-applied on top: - toolbarController owns selection-toolbar visibility, the active surface and focus restoration; ew-editor-doc/ew-editor-wysiwyg activate it from real focus events instead of faking view.hasFocus(). - daCursorPlugin replaces yCursorPlugin, keeping main's collabCursorBuilder and adding the shouldBroadcast predicate. - The selection toolbar renders a reduced section set while the WYSIWYG iframe is the active surface. ew-editor-doc.js and ew-selection-toolbar.js were rebuilt from main because this branch's copies had regressed several main-only features; the six _scrollDocToProseIndex tests that were failing on this branch now pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
handleCursorMove was skipped entirely while the doc view was hidden, so in layout (WYSIWYG-only) view the toolbar only refreshed for non-empty selections — a caret move left Bold/Italic showing the previous position's state. That guard existed only because the handler used to force `view.hasFocus = () => true` (#1302): y-prosemirror's _isLocalCursorInView() re-ran its relative-selection restore on the hidden view and reverted remote edits. It returns false immediately for an unfocused view, so now that the focus lie is gone the guard is redundant — and it is the one thing suppressing the stored-mark sync the toolbar reads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The block toolbar was left orphaned when toolbarController took ownership of toolbar visibility, so selecting a block in layout/wysiwyg view showed nothing (no change variant, add item, replace block or edit-block modal). toolbarController now owns both toolbars: each surface tracks its selected block descriptor and render() arbitrates, showing the block toolbar when a block is selected and the selection toolbar otherwise. The iframe's NODE_SELECT relay reports the block alongside the doc plugin's update, so both surfaces feed the same slots. Clicks inside the block toolbar are excluded from the outside-pointerdown dismissal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…handle Selecting a block with the table select handle in content view stranded the controller with no active surface, after which neither toolbar could show again for the rest of the session — every later text selection was reported against a null surface and dropped. Two causes, both "the doc surface is more than view.dom": - The handle is a plugin widget appended next to the ProseMirror dom, not inside it, so the outside-pointerdown dismissal read clicking it as the user leaving the editor. Hit-test the mount container instead, which also covers the comments gutter and any future editor chrome. - ProseMirror moves DOM focus within its own dom when it installs a NodeSelection, firing focusout. The deferred check only kept the surface alive when focus landed on the selection toolbar, and `document.activeElement` stops at the shadow host so it could not see the prose dom anyway. Bail on the shadow-aware `view.hasFocus()`, and treat the block toolbar as a valid focus destination too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Opening "edit block" from the wysiwyg toolbar gave a modal the toolbar could never appear in: layout mode doesn't serve the doc surface, so every selection made inside the dialog was discarded. The modal also opened unfocused, because re-parenting the ProseMirror dom into the dialog drops DOM focus and the `view.focus()` in enterBlockEdit ran a render too early. The controller now tracks the modal explicitly. While it is open the doc view is the only servable surface regardless of editor mode, the iframe behind the backdrop can't claim the surface back, and the body-hosted block toolbar stays hidden rather than rendering behind the backdrop. ew-editor-doc restores focus after the paint that displays the dialog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the two toolbar markdown docs added by this branch, and cut the comments back to what the code needs: describe the behaviour as it stands rather than the focus hack it replaced. The da-cursor-plugin fork keeps its comments, since they explain why the fork exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://imgswap--da-live--adobe.aem.live/?nx=imgindex
Merge with adobe/da-nx#774