From c4b35f0bfd24058cf084d4cff8307b1f99120b0b Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 22 Jul 2026 16:36:14 -0700 Subject: [PATCH 01/21] feat(canvas): show loose page content in outline panel parseSections now walks all direct children of a section (not just div[class] blocks), grouping contiguous runs of loose content (p, h1-h6, ul/ol, picture) into read-only "Default content" entries so a section with only body text no longer shows as empty. Adds editorProseSelectChange, a generalized position-based select/scroll channel (separate from the block-index-based editorSelectChange), and wires it into ew-editor-doc.js to scroll to an arbitrary prose position when a default-content entry is clicked. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/editor-utils.js | 82 +++++++++++-- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 20 +++- .../ew-page-outline/ew-page-outline.css | 10 ++ .../canvas/ew-page-outline/ew-page-outline.js | 49 +++++--- .../canvas/editor-utils/editor-utils.test.js | 109 ++++++++++++++++++ 5 files changed, 244 insertions(+), 26 deletions(-) diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 42bf61e5b..d208dcceb 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -261,22 +261,69 @@ export function getInstrumentedHTML(view) { const SKIP_BLOCK_CLASSES = new Set(['default-content-wrapper', 'metadata', 'block-marker']); +// A loose (non-block) node counts as "empty" — and is skipped entirely, never even +// breaking/joining a run — when it has no non-whitespace text and no image/media content. +// prose2aem doesn't always strip these (e.g. an empty

can survive serialization +// even though empty top-level

tags are stripped upstream). +function hasLooseContent(el) { + if (el.textContent?.trim()) return true; + return el.matches?.('img') || !!el.querySelector?.('img'); +} + +// Loose text elements (h1-h6, p, ol, ul) get data-prose-index stamped directly on +// themselves by getInstrumentedHTML; images get data-image-index instead. Either +// attribute may live on the node itself (e.g. a top-level 's ) or nested +// inside it (e.g. an inside a wrapping element). +function getLooseProseIndex(el) { + const own = el.getAttribute('data-prose-index') ?? el.getAttribute('data-image-index'); + if (own != null) return Number(own); + const nested = el.querySelector('[data-prose-index], [data-image-index]'); + if (!nested) return undefined; + const attr = nested.getAttribute('data-prose-index') ?? nested.getAttribute('data-image-index'); + return attr != null ? Number(attr) : undefined; +} + export function parseSections(htmlText) { const doc = new DOMParser().parseFromString(htmlText, 'text/html'); const container = doc.querySelector('main') ?? doc.body; let flatIndex = 0; return Array.from(container.querySelectorAll(':scope > div'), (section, sectionIndex) => { const blocks = []; - Array.from(section.querySelectorAll(':scope > div[class]')).forEach((el) => { - const name = el.classList[0]; - if (!name || SKIP_BLOCK_CLASSES.has(name)) return; - const rawProseIndex = el.getAttribute('data-block-index'); - const proseIndex = rawProseIndex != null ? Number(rawProseIndex) : undefined; - const innerText = el.textContent?.trim() ?? ''; - blocks.push({ name, blockIndex: flatIndex, proseIndex, innerText }); - flatIndex += 1; + const items = []; + let currentRun = []; + + const flushRun = () => { + if (currentRun.length) { + items.push({ + type: 'content', + proseIndex: getLooseProseIndex(currentRun[0]), + innerText: currentRun.map((el) => el.textContent.trim()).filter(Boolean).join(' '), + }); + } + currentRun = []; + }; + + Array.from(section.children).forEach((el) => { + const name = el.tagName === 'DIV' ? el.classList[0] : undefined; + const isBlock = name && !SKIP_BLOCK_CLASSES.has(name); + + if (isBlock) { + flushRun(); + const rawProseIndex = el.getAttribute('data-block-index'); + const proseIndex = rawProseIndex != null ? Number(rawProseIndex) : undefined; + const innerText = el.textContent?.trim() ?? ''; + const block = { name, blockIndex: flatIndex, proseIndex, innerText }; + blocks.push(block); + items.push({ type: 'block', ...block }); + flatIndex += 1; + return; + } + + if (hasLooseContent(el)) currentRun.push(el); }); - return { sectionIndex, blocks }; + flushRun(); + + return { sectionIndex, blocks, items }; }); } @@ -333,6 +380,23 @@ export const editorSelectChange = (() => { }; })(); +// Event observable — no replay on subscribe. See docs/canvas-events.md. +// Selects/scrolls to an arbitrary ProseMirror document position (not a block index). +// Used by the outline's default-content entries; general enough for other features +// (e.g. a metadata-editing mode) to reuse for position-based selection. +export const editorProseSelectChange = (() => { + const listeners = new Set(); + return { + emit(detail) { + listeners.forEach((fn) => fn(detail)); + }, + subscribe(fn) { + listeners.add(fn); + return () => listeners.delete(fn); + }, + }; +})(); + export function updateDocument(ctx) { if (ctx.suppressRerender) return undefined; const body = getInstrumentedHTML(ctx.view); diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index 7eed6e97f..bfc88818a 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -1,9 +1,9 @@ import { LitElement, html, nothing } from 'da-lit'; -import { yUndo, yRedo, NodeSelection } from 'da-y-wrapper'; +import { yUndo, yRedo, NodeSelection, TextSelection } from 'da-y-wrapper'; import { getNx } from '../../../scripts/utils.js'; import { updateDocument, updateCursors, getInstrumentedHTML, - editorHtmlChange, editorSelectChange, getEditor, + editorHtmlChange, editorSelectChange, editorProseSelectChange, getEditor, } from '../editor-utils/editor-utils.js'; import { getActiveBlockIndex, getBlockPositions } from '../editor-utils/blocks.js'; import { @@ -118,6 +118,19 @@ export class EwEditorDoc extends LitElement { view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); } + // proseIndex is already a raw doc position (no lookup table needed). Content it + // points at (text or an image) isn't necessarily a whole top-level node, so use + // TextSelection.near — it resolves to the nearest valid selection without throwing. + _scrollDocToProseIndex(proseIndex) { + if (proseIndex == null || proseIndex < 0) return; + const { view } = this._proseContext ?? {}; + if (!view) return; + const { doc } = view.state; + if (proseIndex > doc.content.size) return; + const sel = TextSelection.near(doc.resolve(proseIndex)); + view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); + } + _broadcastSelectedNode(scrollIntoView = false) { const port = this._controllerCtx?.port; const { view } = this._proseContext ?? {}; @@ -300,6 +313,8 @@ export class EwEditorDoc extends LitElement { this._scrollDocToBlock(blockIndex); if (source === 'outline') this._broadcastSelectedNode(true); }); + this._unsubscribeProseSelect = editorProseSelectChange + .subscribe(({ proseIndex }) => this._scrollDocToProseIndex(proseIndex)); this._onCanvasHighlight = (e) => this._applyHighlight(e.detail); document.addEventListener('nx-highlight-selection', this._onCanvasHighlight); } @@ -313,6 +328,7 @@ export class EwEditorDoc extends LitElement { this.parentElement?.removeEventListener('nx-wysiwyg-port-ready', this._onWysiwygPortReady); document.removeEventListener('nx-highlight-selection', this._onCanvasHighlight); this._unsubscribeSelect?.(); + this._unsubscribeProseSelect?.(); this._teardown(); setSelectionToolbarCtx(); super.disconnectedCallback(); diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.css b/blocks/canvas/ew-page-outline/ew-page-outline.css index 926fb37f6..97d894ce0 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.css +++ b/blocks/canvas/ew-page-outline/ew-page-outline.css @@ -116,6 +116,16 @@ font-weight: 400; } +.content-item { + cursor: pointer; +} + +.content-label { + font-weight: 400; + font-style: italic; + color: var(--s2-gray-700); +} + .block-name { flex: 1; overflow: hidden; diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index cebfda450..285f07bd6 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -1,7 +1,7 @@ import { LitElement, html, nothing } from 'da-lit'; import { getNx } from '../../../scripts/utils.js'; import { treeKeydown } from '../utils/tree-nav.js'; -import { editorHtmlChange, editorSelectChange, parseSections } from '../editor-utils/editor-utils.js'; +import { editorHtmlChange, editorSelectChange, editorProseSelectChange, parseSections } from '../editor-utils/editor-utils.js'; import { getExtensionsBridge } from '../editor-utils/extensions-bridge.js'; import { deleteBlock, @@ -30,13 +30,21 @@ const DROP_POSITIONS = { AFTER: 'after', }; +function itemsEqual(item, other) { + if (!other || item.type !== other.type) return false; + if (item.type === 'block') return item.blockIndex === other.blockIndex && item.name === other.name; + return item.proseIndex === other.proseIndex && item.innerText === other.innerText; +} + function sectionsEqual(a, b) { if (!a || !b || a.length !== b.length) return false; return a.every((sec, i) => { const other = b[i]; return sec.sectionIndex === other.sectionIndex && sec.blocks.length === other.blocks.length - && sec.blocks.every((blk, j) => blk.name === other.blocks[j].name); + && sec.blocks.every((blk, j) => blk.name === other.blocks[j].name) + && sec.items.length === other.items.length + && sec.items.every((item, j) => itemsEqual(item, other.items[j])); }); } @@ -108,6 +116,10 @@ class EwPageOutline extends LitElement { editorSelectChange.emit({ blockIndex, source: 'outline' }); } + _selectProse(proseIndex) { + editorProseSelectChange.emit({ proseIndex }); + } + _clearDropIndicator() { this.shadowRoot.querySelector('[data-drop-position]')?.removeAttribute('data-drop-position'); } @@ -263,28 +275,35 @@ class EwPageOutline extends LitElement {

`; } diff --git a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js index ad4484854..3fd147183 100644 --- a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js +++ b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js @@ -5,11 +5,13 @@ setNx('/test/fixtures/nx', { hostname: 'example.com' }); let getPreviewOrigin; let fetchWysiwygBranch; +let parseSections; before(async () => { const mod = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js'); getPreviewOrigin = mod.getPreviewOrigin; fetchWysiwygBranch = mod.fetchWysiwygBranch; + parseSections = mod.parseSections; }); describe('getPreviewOrigin', () => { @@ -119,3 +121,110 @@ describe('fetchWysiwygBranch', () => { expect(branch).to.equal('valid'); }); }); + +describe('parseSections', () => { + it('collects a single block and mirrors it in items (unchanged behavior)', () => { + const html = `
+
Hero content
+
`; + const [section] = parseSections(html); + expect(section.blocks).to.deep.equal([ + { name: 'hero', blockIndex: 0, proseIndex: 0, innerText: 'Hero content' }, + ]); + expect(section.items).to.deep.equal([ + { type: 'block', name: 'hero', blockIndex: 0, proseIndex: 0, innerText: 'Hero content' }, + ]); + }); + + it('returns empty blocks and items for a section with nothing in it', () => { + const html = '
'; + const [section] = parseSections(html); + expect(section.blocks).to.deep.equal([]); + expect(section.items).to.deep.equal([]); + }); + + it('produces separate default-content entries before and after a block', () => { + const html = `
+

Intro text

+
Hero
+

Outro text

+
`; + const [section] = parseSections(html); + expect(section.items.map((i) => i.type)).to.deep.equal(['content', 'block', 'content']); + expect(section.items[0]).to.deep.equal({ type: 'content', proseIndex: 1, innerText: 'Intro text' }); + expect(section.items[2]).to.deep.equal({ type: 'content', proseIndex: 20, innerText: 'Outro text' }); + }); + + it('groups consecutive loose children into a single content entry', () => { + const html = `
+

Title

+

Para one

+

Para two

+
`; + const [section] = parseSections(html); + expect(section.items).to.deep.equal([ + { type: 'content', proseIndex: 1, innerText: 'Title Para one Para two' }, + ]); + }); + + it('treats empty loose nodes as invisible — they neither break nor join a run', () => { + const html = `
+

Para one

+

+

+

Para two

+
`; + const [section] = parseSections(html); + expect(section.items).to.deep.equal([ + { type: 'content', proseIndex: 1, innerText: 'Para one Para two' }, + ]); + }); + + it('produces nothing for a run made up entirely of empty nodes', () => { + const html = `
+
Hero
+

+

+
`; + const [section] = parseSections(html); + expect(section.items).to.have.length(1); + expect(section.items[0].type).to.equal('block'); + }); + + it('reads proseIndex from data-image-index on a loose image', () => { + const html = `
+ +
`; + const [section] = parseSections(html); + expect(section.items).to.deep.equal([ + { type: 'content', proseIndex: 7, innerText: '' }, + ]); + }); + + it('takes proseIndex from the first non-empty node in a run', () => { + const html = `
+

+

First real content

+

More content

+
`; + const [section] = parseSections(html); + expect(section.items).to.deep.equal([ + { type: 'content', proseIndex: 9, innerText: 'First real content More content' }, + ]); + }); + + it('handles multiple sections independently', () => { + const html = `
+

Section one text

+
Cards
+
`; + const sections = parseSections(html); + expect(sections).to.have.length(2); + expect(sections[0].items).to.deep.equal([ + { type: 'content', proseIndex: 1, innerText: 'Section one text' }, + ]); + expect(sections[1].items).to.deep.equal([ + { type: 'block', name: 'cards', blockIndex: 0, proseIndex: 0, innerText: 'Cards' }, + ]); + }); +}); From 9475d4e55abf6649cb24a825d7b7530ec0bee9f3 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Thu, 23 Jul 2026 09:10:12 -0700 Subject: [PATCH 02/21] feat(canvas): expand default content into paragraph/heading/image/list children Default content in the outline is now a collapsed-by-default group listing each consecutive loose item's kind, instead of one opaque entry. Clicking an image child also selects it as a NodeSelection and broadcasts it to the layout-view iframe, matching block selection; text kinds stay doc-view only pending a cross-repo follow-up. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/editor-utils.js | 25 +++++ blocks/canvas/ew-editor-doc/ew-editor-doc.js | 14 ++- .../ew-page-outline/ew-page-outline.css | 28 +++++ .../canvas/ew-page-outline/ew-page-outline.js | 62 ++++++++-- .../canvas/editor-utils/editor-utils.test.js | 105 ++++++++++++++--- .../ew-page-outline/ew-page-outline.test.js | 106 ++++++++++++++++++ 6 files changed, 311 insertions(+), 29 deletions(-) create mode 100644 test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index d208dcceb..2d92c3132 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -283,6 +283,25 @@ function getLooseProseIndex(el) { return attr != null ? Number(attr) : undefined; } +// Classifies a loose top-level node for display in the outline's expanded +// "Default content" group. Anything that isn't a recognized text tag (e.g. a +// or bare ) is treated as an image — hasLooseContent() only lets +// through text-bearing nodes or ones containing an . +// +// The image schema node is inline-only, so it always lives inside a block node. +// prose2aem unwraps that wrapper down to a bare only when the image is +// the sole content of its section (see makePictures() in prose2aem.js); otherwise +// the

survives with the picture nested inside it. A

with no text of its +// own — only an image — is really an image, not a paragraph. +function getLooseNodeKind(el) { + const tag = el.tagName; + if (/^H[1-6]$/.test(tag)) return { kind: 'heading', level: Number(tag[1]) }; + if (tag === 'OL') return { kind: 'list', ordered: true }; + if (tag === 'UL') return { kind: 'list', ordered: false }; + if (tag === 'P') return { kind: el.textContent?.trim() ? 'paragraph' : 'image' }; + return { kind: 'image' }; +} + export function parseSections(htmlText) { const doc = new DOMParser().parseFromString(htmlText, 'text/html'); const container = doc.querySelector('main') ?? doc.body; @@ -298,6 +317,12 @@ export function parseSections(htmlText) { type: 'content', proseIndex: getLooseProseIndex(currentRun[0]), innerText: currentRun.map((el) => el.textContent.trim()).filter(Boolean).join(' '), + children: currentRun.map((el) => ({ + type: 'content', + ...getLooseNodeKind(el), + proseIndex: getLooseProseIndex(el), + innerText: el.textContent.trim(), + })), }); } currentRun = []; diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index bfc88818a..47d4e29bf 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -121,12 +121,22 @@ export class EwEditorDoc extends LitElement { // proseIndex is already a raw doc position (no lookup table needed). Content it // points at (text or an image) isn't necessarily a whole top-level node, so use // TextSelection.near — it resolves to the nearest valid selection without throwing. - _scrollDocToProseIndex(proseIndex) { + // An image is the exception: it's a real node, so select it the same way a block + // is (NodeSelection + broadcast) to get layout-view parity via the quick-edit port. + _scrollDocToProseIndex(proseIndex, kind) { if (proseIndex == null || proseIndex < 0) return; const { view } = this._proseContext ?? {}; if (!view) return; const { doc } = view.state; if (proseIndex > doc.content.size) return; + + if (kind === 'image' && doc.nodeAt(proseIndex)?.type.name === 'image') { + const sel = NodeSelection.create(doc, proseIndex); + view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); + this._broadcastSelectedNode(true); + return; + } + const sel = TextSelection.near(doc.resolve(proseIndex)); view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); } @@ -314,7 +324,7 @@ export class EwEditorDoc extends LitElement { if (source === 'outline') this._broadcastSelectedNode(true); }); this._unsubscribeProseSelect = editorProseSelectChange - .subscribe(({ proseIndex }) => this._scrollDocToProseIndex(proseIndex)); + .subscribe(({ proseIndex, kind }) => this._scrollDocToProseIndex(proseIndex, kind)); this._onCanvasHighlight = (e) => this._applyHighlight(e.detail); document.addEventListener('nx-highlight-selection', this._onCanvasHighlight); } diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.css b/blocks/canvas/ew-page-outline/ew-page-outline.css index 97d894ce0..2aac50f08 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.css +++ b/blocks/canvas/ew-page-outline/ew-page-outline.css @@ -116,10 +116,38 @@ font-weight: 400; } +.content-group { + margin: 0; + padding: 0; +} + .content-item { cursor: pointer; } +.content-item[aria-expanded]::before { + content: '›'; + display: inline-block; + flex-shrink: 0; + width: 1rem; + text-align: center; + transition: transform 0.15s; +} + +.content-item[aria-expanded="true"]::before { + transform: rotate(90deg); +} + +.content-children { + list-style: none; + margin: 0; + padding: 0; +} + +.content-child { + padding-inline-start: var(--s2-spacing-500); +} + .content-label { font-weight: 400; font-style: italic; diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 285f07bd6..cd5264e3a 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -30,10 +30,28 @@ const DROP_POSITIONS = { AFTER: 'after', }; +function contentChildEqual(child, other) { + return child.proseIndex === other.proseIndex && child.innerText === other.innerText + && child.kind === other.kind && child.level === other.level && child.ordered === other.ordered; +} + +function contentChildLabel(child) { + switch (child.kind) { + case 'heading': return `Heading ${child.level}`; + case 'list': return child.ordered ? 'Numbered list' : 'Bulleted list'; + case 'image': return 'Image'; + default: return 'Paragraph'; + } +} + function itemsEqual(item, other) { if (!other || item.type !== other.type) return false; if (item.type === 'block') return item.blockIndex === other.blockIndex && item.name === other.name; - return item.proseIndex === other.proseIndex && item.innerText === other.innerText; + if (item.proseIndex !== other.proseIndex || item.innerText !== other.innerText) return false; + const children = item.children ?? []; + const otherChildren = other.children ?? []; + return children.length === otherChildren.length + && children.every((child, i) => contentChildEqual(child, otherChildren[i])); } function sectionsEqual(a, b) { @@ -54,11 +72,13 @@ class EwPageOutline extends LitElement { _selectedBlockIndex: { state: true }, _hashState: { state: true }, _hasBlockLibrary: { state: true }, + _expandedContent: { state: true }, }; connectedCallback() { super.connectedCallback(); this.shadowRoot.adoptedStyleSheets = [style]; + this._expandedContent = new Set(); this._unsubHash = hashChange.subscribe((state) => { this._hashState = state; }); this._unsubscribeHtml = editorHtmlChange.subscribe((aemHtml) => { if (aemHtml.trim()) { @@ -116,8 +136,15 @@ class EwPageOutline extends LitElement { editorSelectChange.emit({ blockIndex, source: 'outline' }); } - _selectProse(proseIndex) { - editorProseSelectChange.emit({ proseIndex }); + _selectProse(proseIndex, kind) { + editorProseSelectChange.emit({ proseIndex, kind }); + } + + _toggleContentGroup(key) { + const next = new Set(this._expandedContent); + if (next.has(key)) next.delete(key); + else next.add(key); + this._expandedContent = next; } _clearDropIndicator() { @@ -248,6 +275,28 @@ class EwPageOutline extends LitElement { `; } + _renderContentGroup(item, isFirst) { + const key = item.proseIndex; + const expanded = this._expandedContent?.has(key); + return html` +

  • +
    this._toggleContentGroup(key)}> + +
    + ${expanded ? html` +
      + ${item.children.map((child) => html` +
    • { e.stopPropagation(); this._selectProse(child.proseIndex, child.kind); }}> + +
    • `)} +
    ` : nothing} +
  • `; + } + _renderSection(sec, isFirstSection) { return html`
  • ` - : html` -
  • this._selectProse(item.proseIndex)}> - -
  • `))} + : this._renderContentGroup(item, isFirstSection && itemIdx === 0)))} `; } diff --git a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js index 3fd147183..b16bc7be1 100644 --- a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js +++ b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js @@ -151,20 +151,71 @@ describe('parseSections', () => { `; const [section] = parseSections(html); expect(section.items.map((i) => i.type)).to.deep.equal(['content', 'block', 'content']); - expect(section.items[0]).to.deep.equal({ type: 'content', proseIndex: 1, innerText: 'Intro text' }); - expect(section.items[2]).to.deep.equal({ type: 'content', proseIndex: 20, innerText: 'Outro text' }); + expect(section.items[0]).to.deep.equal({ + type: 'content', + proseIndex: 1, + innerText: 'Intro text', + children: [{ type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Intro text' }], + }); + expect(section.items[2]).to.deep.equal({ + type: 'content', + proseIndex: 20, + innerText: 'Outro text', + children: [{ type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Outro text' }], + }); }); - it('groups consecutive loose children into a single content entry', () => { + it('groups consecutive loose children into a single content entry, listing each child', () => { const html = `

    Title

    Para one

    Para two

    `; const [section] = parseSections(html); - expect(section.items).to.deep.equal([ - { type: 'content', proseIndex: 1, innerText: 'Title Para one Para two' }, - ]); + expect(section.items).to.deep.equal([{ + type: 'content', + proseIndex: 1, + innerText: 'Title Para one Para two', + children: [ + { type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title' }, + { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one' }, + { type: 'content', kind: 'paragraph', proseIndex: 12, innerText: 'Para two' }, + ], + }]); + }); + + it('classifies ordered/unordered lists and images', () => { + const html = `
    +
    1. one
    +
    • two
    + +
    `; + const [section] = parseSections(html); + const [{ children }] = section.items; + expect(children.map((c) => c.kind)).to.deep.equal(['list', 'list', 'image']); + expect(children[0].ordered).to.equal(true); + expect(children[1].ordered).to.equal(false); + }); + + it('classifies a

    -wrapped image as image, not paragraph (prose2aem only unwraps the

    when the image is the section\'s sole child)', () => { + const html = `

    +

    Title

    +

    +

    Caption text

    +
    `; + const [section] = parseSections(html); + const [{ children }] = section.items; + expect(children.map((c) => c.kind)).to.deep.equal(['heading', 'image', 'paragraph']); + expect(children[1].proseIndex).to.equal(5); + }); + + it('keeps a paragraph with mixed text and an inline image classified as paragraph', () => { + const html = `
    +

    Some text more text

    +
    `; + const [section] = parseSections(html); + const [{ children }] = section.items; + expect(children[0].kind).to.equal('paragraph'); }); it('treats empty loose nodes as invisible — they neither break nor join a run', () => { @@ -175,9 +226,15 @@ describe('parseSections', () => {

    Para two

    `; const [section] = parseSections(html); - expect(section.items).to.deep.equal([ - { type: 'content', proseIndex: 1, innerText: 'Para one Para two' }, - ]); + expect(section.items).to.deep.equal([{ + type: 'content', + proseIndex: 1, + innerText: 'Para one Para two', + children: [ + { type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Para one' }, + { type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Para two' }, + ], + }]); }); it('produces nothing for a run made up entirely of empty nodes', () => { @@ -196,9 +253,12 @@ describe('parseSections', () => { `; const [section] = parseSections(html); - expect(section.items).to.deep.equal([ - { type: 'content', proseIndex: 7, innerText: '' }, - ]); + expect(section.items).to.deep.equal([{ + type: 'content', + proseIndex: 7, + innerText: '', + children: [{ type: 'content', kind: 'image', proseIndex: 7, innerText: '' }], + }]); }); it('takes proseIndex from the first non-empty node in a run', () => { @@ -208,9 +268,15 @@ describe('parseSections', () => {

    More content

    `; const [section] = parseSections(html); - expect(section.items).to.deep.equal([ - { type: 'content', proseIndex: 9, innerText: 'First real content More content' }, - ]); + expect(section.items).to.deep.equal([{ + type: 'content', + proseIndex: 9, + innerText: 'First real content More content', + children: [ + { type: 'content', kind: 'paragraph', proseIndex: 9, innerText: 'First real content' }, + { type: 'content', kind: 'paragraph', proseIndex: 15, innerText: 'More content' }, + ], + }]); }); it('handles multiple sections independently', () => { @@ -220,9 +286,12 @@ describe('parseSections', () => { `; const sections = parseSections(html); expect(sections).to.have.length(2); - expect(sections[0].items).to.deep.equal([ - { type: 'content', proseIndex: 1, innerText: 'Section one text' }, - ]); + expect(sections[0].items).to.deep.equal([{ + type: 'content', + proseIndex: 1, + innerText: 'Section one text', + children: [{ type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Section one text' }], + }]); expect(sections[1].items).to.deep.equal([ { type: 'block', name: 'cards', blockIndex: 0, proseIndex: 0, innerText: 'Cards' }, ]); diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js new file mode 100644 index 000000000..a0443f930 --- /dev/null +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -0,0 +1,106 @@ +/* eslint-disable no-underscore-dangle */ +import { expect } from '@esm-bundle/chai'; +import { setNx } from '../../../../../scripts/utils.js'; + +setNx('/test/fixtures/nx', { hostname: 'example.com' }); + +let editorProseSelectChange; + +before(async () => { + await import('../../../../../blocks/canvas/ew-page-outline/ew-page-outline.js'); + ({ editorProseSelectChange } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); +}); + +async function createOutline() { + const el = document.createElement('ew-page-outline'); + // _checkBlockLibrary fires once a hash with org/site is set — no-op it so this + // test doesn't reach the network. + el._checkBlockLibrary = async () => {}; + document.body.appendChild(el); + await el.updateComplete; + el._hashState = { org: 'org', site: 'site', path: 'page' }; + await el.updateComplete; + return el; +} + +const contentGroupItem = (proseIndex, children) => ({ + type: 'content', + proseIndex, + innerText: children.map((c) => c.innerText).filter(Boolean).join(' '), + children, +}); + +describe('ew-page-outline — expandable default content', () => { + let el; + + beforeEach(async () => { + el = await createOutline(); + el._sections = [{ + sectionIndex: 0, + blocks: [], + items: [ + contentGroupItem(1, [ + { type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title' }, + { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one' }, + { type: 'content', kind: 'image', proseIndex: 9, innerText: '' }, + { type: 'content', kind: 'list', ordered: true, proseIndex: 12, innerText: 'one two' }, + ]), + ], + }]; + await el.updateComplete; + }); + + afterEach(() => { el.remove(); }); + + it('renders a single collapsed "Default content" row with no children visible', () => { + const header = el.shadowRoot.querySelector('.content-item'); + expect(header).to.exist; + expect(header.textContent.trim()).to.equal('Default content'); + expect(header.getAttribute('aria-expanded')).to.equal('false'); + expect(el.shadowRoot.querySelector('.content-children')).to.be.null; + }); + + it('expands to list every consecutive item on header click, then collapses again', async () => { + const header = el.shadowRoot.querySelector('.content-item'); + header.click(); + await el.updateComplete; + + expect(header.getAttribute('aria-expanded')).to.equal('true'); + const children = [...el.shadowRoot.querySelectorAll('.content-child')]; + expect(children).to.have.lengthOf(4); + expect(children.map((c) => c.textContent.trim())).to.deep.equal([ + 'Heading 2', 'Paragraph', 'Image', 'Numbered list', + ]); + + header.click(); + await el.updateComplete; + expect(header.getAttribute('aria-expanded')).to.equal('false'); + expect(el.shadowRoot.querySelector('.content-children')).to.be.null; + }); + + it('emits editorProseSelectChange with the child\'s own proseIndex and kind on click', async () => { + el.shadowRoot.querySelector('.content-item').click(); + await el.updateComplete; + + let received; + const unsub = editorProseSelectChange.subscribe((detail) => { received = detail; }); + const paragraphChild = [...el.shadowRoot.querySelectorAll('.content-child')][1]; + paragraphChild.click(); + unsub(); + + expect(received).to.deep.equal({ proseIndex: 5, kind: 'paragraph' }); + }); + + it('emits the image kind for an image child, enabling layout-view NodeSelection', async () => { + el.shadowRoot.querySelector('.content-item').click(); + await el.updateComplete; + + let received; + const unsub = editorProseSelectChange.subscribe((detail) => { received = detail; }); + const imageChild = [...el.shadowRoot.querySelectorAll('.content-child')][2]; + imageChild.click(); + unsub(); + + expect(received).to.deep.equal({ proseIndex: 9, kind: 'image' }); + }); +}); From 20939cdbf9a41cd7b565a022e0efd5e6c98e9897 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Thu, 23 Jul 2026 09:19:31 -0700 Subject: [PATCH 03/21] fix(canvas): rerender on block identity changes; add code blocks to outline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changing a heading's level (or list ordered/unordered) only nudged the WYSIWYG mini-editor via getEditor(), never emitting editorHtmlChange — so the outline (and other listeners) never re-parsed. createTrackingPlugin now forces a full rerenderPage() whenever a change is 'attrs' or 'replaced', reserving the lightweight sync path for plain text edits. Also recognizes
     code blocks in the outline's default-content group
    (stamped with data-prose-index like other loose nodes), and renames
    "Bulleted list" to "Bullet list" to match the slash-command menu wording.
    
    Co-Authored-By: Claude Sonnet 5 
    ---
     blocks/canvas/editor-utils/editor-utils.js    |  2 +
     blocks/canvas/editor-utils/prose-diff.js      |  9 ++++-
     .../canvas/ew-page-outline/ew-page-outline.js |  3 +-
     .../canvas/editor-utils/editor-utils.test.js  | 11 +++++
     .../canvas/editor-utils/prose-diff.test.js    | 40 +++++++++++++++++++
     .../ew-page-outline/ew-page-outline.test.js   |  5 ++-
     6 files changed, 66 insertions(+), 4 deletions(-)
    
    diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js
    index 2d92c3132..1409219db 100644
    --- a/blocks/canvas/editor-utils/editor-utils.js
    +++ b/blocks/canvas/editor-utils/editor-utils.js
    @@ -133,6 +133,7 @@ const EDITABLES = [
       { selector: 'p', nodeName: 'P' },
       { selector: 'ol', nodeName: 'OL' },
       { selector: 'ul', nodeName: 'UL' },
    +  { selector: 'pre', nodeName: 'PRE' },
     ];
     const EDITABLE_SELECTORS = EDITABLES.map((edit) => edit.selector).join(', ');
     
    @@ -298,6 +299,7 @@ function getLooseNodeKind(el) {
       if (/^H[1-6]$/.test(tag)) return { kind: 'heading', level: Number(tag[1]) };
       if (tag === 'OL') return { kind: 'list', ordered: true };
       if (tag === 'UL') return { kind: 'list', ordered: false };
    +  if (tag === 'PRE') return { kind: 'code' };
       if (tag === 'P') return { kind: el.textContent?.trim() ? 'paragraph' : 'image' };
       return { kind: 'image' };
     }
    diff --git a/blocks/canvas/editor-utils/prose-diff.js b/blocks/canvas/editor-utils/prose-diff.js
    index fe8a9259a..64326cea1 100644
    --- a/blocks/canvas/editor-utils/prose-diff.js
    +++ b/blocks/canvas/editor-utils/prose-diff.js
    @@ -163,7 +163,14 @@ export function createTrackingPlugin(rerenderPage, updateCursors, getEditor, onS
                 const changes = findChangedNodes(prevState.doc, view.state.doc);
     
                 if (changes.length > 0) {
    -              const commonEditable = findCommonEditableAncestor(view, changes, prevState);
    +              // An 'attrs' or 'replaced' change alters a block's own identity (e.g. a
    +              // heading's level, or switching bullet_list <-> ordered_list) — the
    +              // outline and other editorHtmlChange consumers need a full re-parse for
    +              // that, not the in-place text sync a plain edit gets.
    +              const identityChanged = changes.some((c) => c.type === 'attrs' || c.type === 'replaced');
    +              const commonEditable = identityChanged
    +                ? null
    +                : findCommonEditableAncestor(view, changes, prevState);
     
                   if (commonEditable) {
                     getEditor?.({ cursorOffset: commonEditable.pos + 1 });
    diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js
    index cd5264e3a..0afe30270 100644
    --- a/blocks/canvas/ew-page-outline/ew-page-outline.js
    +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js
    @@ -38,8 +38,9 @@ function contentChildEqual(child, other) {
     function contentChildLabel(child) {
       switch (child.kind) {
         case 'heading': return `Heading ${child.level}`;
    -    case 'list': return child.ordered ? 'Numbered list' : 'Bulleted list';
    +    case 'list': return child.ordered ? 'Numbered list' : 'Bullet list';
         case 'image': return 'Image';
    +    case 'code': return 'Code block';
         default: return 'Paragraph';
       }
     }
    diff --git a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js
    index b16bc7be1..5eb53869c 100644
    --- a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js
    +++ b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js
    @@ -218,6 +218,17 @@ describe('parseSections', () => {
         expect(children[0].kind).to.equal('paragraph');
       });
     
    +  it('classifies a 
     as a code block', () => {
    +    const html = `
    +

    Title

    +
    const x = 1;
    +
    `; + const [section] = parseSections(html); + const [{ children }] = section.items; + expect(children.map((c) => c.kind)).to.deep.equal(['heading', 'code']); + expect(children[1]).to.deep.equal({ type: 'content', kind: 'code', proseIndex: 5, innerText: 'const x = 1;' }); + }); + it('treats empty loose nodes as invisible — they neither break nor join a run', () => { const html = `

    Para one

    diff --git a/test/unit/blocks/canvas/editor-utils/prose-diff.test.js b/test/unit/blocks/canvas/editor-utils/prose-diff.test.js index 46aeebfdf..464431f91 100644 --- a/test/unit/blocks/canvas/editor-utils/prose-diff.test.js +++ b/test/unit/blocks/canvas/editor-utils/prose-diff.test.js @@ -10,6 +10,11 @@ function docWithParagraph(text) { return schema.nodes.doc.create(null, para); } +function docWithHeading(text, level = 2) { + const heading = schema.nodes.heading.create({ level }, schema.text(text)); + return schema.nodes.doc.create(null, heading); +} + function setup() { let rerenderCalls = 0; let getEditorCalls = 0; @@ -56,3 +61,38 @@ describe('createTrackingPlugin — trackingPluginKey skip flag', () => { expect(counts()).to.deep.equal({ rerenderCalls: 1, getEditorCalls: 0 }); }); }); + +function setupHeading(level = 2) { + let rerenderCalls = 0; + let getEditorCalls = 0; + const plugin = createTrackingPlugin( + () => { rerenderCalls += 1; }, + undefined, + () => { getEditorCalls += 1; }, + undefined, + ); + const prevState = EditorState.create({ schema, doc: docWithHeading('Title', level), plugins: [plugin] }); + return { plugin, prevState, counts: () => ({ rerenderCalls, getEditorCalls }) }; +} + +describe('createTrackingPlugin — block identity changes', () => { + it('changing a heading\'s level calls rerenderPage, not getEditor (outline needs a full re-parse)', () => { + const { plugin, prevState, counts } = setupHeading(2); + const tr = prevState.tr.setNodeMarkup(0, undefined, { level: 3 }); + const nextState = prevState.apply(tr); + + plugin.spec.view().update({ state: nextState }, prevState); + + expect(counts()).to.deep.equal({ rerenderCalls: 1, getEditorCalls: 0 }); + }); + + it('a plain text edit inside a heading still takes the lightweight getEditor path', () => { + const { plugin, prevState, counts } = setupHeading(2); + const tr = prevState.tr.insertText('!', 1); + const nextState = prevState.apply(tr); + + plugin.spec.view().update({ state: nextState }, prevState); + + expect(counts()).to.deep.equal({ rerenderCalls: 0, getEditorCalls: 1 }); + }); +}); diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index a0443f930..66053283d 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -44,6 +44,7 @@ describe('ew-page-outline — expandable default content', () => { { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one' }, { type: 'content', kind: 'image', proseIndex: 9, innerText: '' }, { type: 'content', kind: 'list', ordered: true, proseIndex: 12, innerText: 'one two' }, + { type: 'content', kind: 'code', proseIndex: 15, innerText: 'const x = 1;' }, ]), ], }]; @@ -67,9 +68,9 @@ describe('ew-page-outline — expandable default content', () => { expect(header.getAttribute('aria-expanded')).to.equal('true'); const children = [...el.shadowRoot.querySelectorAll('.content-child')]; - expect(children).to.have.lengthOf(4); + expect(children).to.have.lengthOf(5); expect(children.map((c) => c.textContent.trim())).to.deep.equal([ - 'Heading 2', 'Paragraph', 'Image', 'Numbered list', + 'Heading 2', 'Paragraph', 'Image', 'Numbered list', 'Code block', ]); header.click(); From 3e6f0d22e7f509c319306ba14b67858a24eaebd7 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Thu, 23 Jul 2026 10:02:55 -0700 Subject: [PATCH 04/21] fix(canvas): address outline default-content review feedback - getDefaultContentProseIndex: for kind 'image', prefer the nested data-image-index over a wrapping

    's own data-prose-index so the image node resolves correctly in the doc - getDefaultContentKind: classify

    as 'quote' and make the fallback content-aware so text-bearing tags aren't mislabeled 'image'; drop the now-redundant

    branch - prose-diff: narrow the full-rerender trigger to attrs/replaced changes on EDITABLE_TYPES nodes; image/table attr edits take the lightweight getEditor sync path - ew-page-outline: drop redundant blocks comparison from sectionsEqual - rename loose* helpers to defaultContent* to match the AEM term - trim verbose comments to why-only, matching file conventions - add coverage: _scrollDocToProseIndex, blockquote classification, image proseIndex, and image-attr rerender path Co-Authored-By: Claude Opus 4.8 (1M context) --- blocks/canvas/editor-utils/editor-utils.js | 63 +++++----- blocks/canvas/editor-utils/prose-diff.js | 18 ++- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 8 +- .../canvas/ew-page-outline/ew-page-outline.js | 3 +- .../canvas/editor-utils/editor-utils.test.js | 16 ++- .../canvas/editor-utils/prose-diff.test.js | 28 +++++ .../ew-editor-doc/ew-editor-doc.test.js | 112 ++++++++++++++++++ 7 files changed, 200 insertions(+), 48 deletions(-) create mode 100644 test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 1409219db..56c81b8da 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -262,20 +262,19 @@ export function getInstrumentedHTML(view) { const SKIP_BLOCK_CLASSES = new Set(['default-content-wrapper', 'metadata', 'block-marker']); -// A loose (non-block) node counts as "empty" — and is skipped entirely, never even -// breaking/joining a run — when it has no non-whitespace text and no image/media content. -// prose2aem doesn't always strip these (e.g. an empty

    can survive serialization -// even though empty top-level

    tags are stripped upstream). -function hasLooseContent(el) { +function hasDefaultContent(el) { if (el.textContent?.trim()) return true; return el.matches?.('img') || !!el.querySelector?.('img'); } -// Loose text elements (h1-h6, p, ol, ul) get data-prose-index stamped directly on -// themselves by getInstrumentedHTML; images get data-image-index instead. Either -// attribute may live on the node itself (e.g. a top-level 's ) or nested -// inside it (e.g. an inside a wrapping element). -function getLooseProseIndex(el) { +function getDefaultContentProseIndex(el, kind) { + // A

    wrapping an image keeps its own data-prose-index, but only the nested + // data-image-index resolves to the image node (prose2aem leaves the

    unless the + // image is the section's sole child), so for kind 'image' it must win. + if (kind === 'image') { + const nestedImage = el.querySelector('[data-image-index]'); + if (nestedImage) return Number(nestedImage.getAttribute('data-image-index')); + } const own = el.getAttribute('data-prose-index') ?? el.getAttribute('data-image-index'); if (own != null) return Number(own); const nested = el.querySelector('[data-prose-index], [data-image-index]'); @@ -284,24 +283,16 @@ function getLooseProseIndex(el) { return attr != null ? Number(attr) : undefined; } -// Classifies a loose top-level node for display in the outline's expanded -// "Default content" group. Anything that isn't a recognized text tag (e.g. a -// or bare ) is treated as an image — hasLooseContent() only lets -// through text-bearing nodes or ones containing an . -// -// The image schema node is inline-only, so it always lives inside a block node. -// prose2aem unwraps that wrapper down to a bare only when the image is -// the sole content of its section (see makePictures() in prose2aem.js); otherwise -// the

    survives with the picture nested inside it. A

    with no text of its -// own — only an image — is really an image, not a paragraph. -function getLooseNodeKind(el) { +function getDefaultContentKind(el) { const tag = el.tagName; if (/^H[1-6]$/.test(tag)) return { kind: 'heading', level: Number(tag[1]) }; if (tag === 'OL') return { kind: 'list', ordered: true }; if (tag === 'UL') return { kind: 'list', ordered: false }; if (tag === 'PRE') return { kind: 'code' }; - if (tag === 'P') return { kind: el.textContent?.trim() ? 'paragraph' : 'image' }; - return { kind: 'image' }; + if (tag === 'BLOCKQUOTE') return { kind: 'quote' }; + // a text-less

    wraps only an image, as does a bare /; + // anything with text is a paragraph + return { kind: el.textContent?.trim() ? 'paragraph' : 'image' }; } export function parseSections(htmlText) { @@ -317,14 +308,17 @@ export function parseSections(htmlText) { if (currentRun.length) { items.push({ type: 'content', - proseIndex: getLooseProseIndex(currentRun[0]), + proseIndex: getDefaultContentProseIndex(currentRun[0]), innerText: currentRun.map((el) => el.textContent.trim()).filter(Boolean).join(' '), - children: currentRun.map((el) => ({ - type: 'content', - ...getLooseNodeKind(el), - proseIndex: getLooseProseIndex(el), - innerText: el.textContent.trim(), - })), + children: currentRun.map((el) => { + const kindInfo = getDefaultContentKind(el); + return { + type: 'content', + ...kindInfo, + proseIndex: getDefaultContentProseIndex(el, kindInfo.kind), + innerText: el.textContent.trim(), + }; + }), }); } currentRun = []; @@ -346,7 +340,9 @@ export function parseSections(htmlText) { return; } - if (hasLooseContent(el)) currentRun.push(el); + // Skip empty nodes — prose2aem doesn't always strip them (e.g. an empty

    can + // survive serialization) — so they neither break nor join a run. + if (hasDefaultContent(el)) currentRun.push(el); }); flushRun(); @@ -408,9 +404,8 @@ export const editorSelectChange = (() => { })(); // Event observable — no replay on subscribe. See docs/canvas-events.md. -// Selects/scrolls to an arbitrary ProseMirror document position (not a block index). -// Used by the outline's default-content entries; general enough for other features -// (e.g. a metadata-editing mode) to reuse for position-based selection. +// Selects/scrolls to a raw ProseMirror position (not a block index); used by the +// outline's default-content entries. export const editorProseSelectChange = (() => { const listeners = new Set(); return { diff --git a/blocks/canvas/editor-utils/prose-diff.js b/blocks/canvas/editor-utils/prose-diff.js index 64326cea1..7c7df61fa 100644 --- a/blocks/canvas/editor-utils/prose-diff.js +++ b/blocks/canvas/editor-utils/prose-diff.js @@ -54,6 +54,7 @@ export function findChangedNodes(oldDoc, newDoc) { pos, oldAttrs: oldNode.attrs, newAttrs: newNode.attrs, + nodeType: newNode.type.name, }); } @@ -103,6 +104,12 @@ export function findChangedNodes(oldDoc, newDoc) { export const EDITABLE_TYPES = ['heading', 'paragraph', 'ordered_list', 'bullet_list']; +function changedNodeType(change) { + if (change.type === 'attrs') return change.nodeType; + if (change.type === 'replaced') return change.newNode?.type.name ?? change.oldNode?.type.name; + return undefined; +} + export function findCommonEditableAncestor(view, changes, prevState) { if (changes.length === 0) return null; @@ -163,11 +170,12 @@ export function createTrackingPlugin(rerenderPage, updateCursors, getEditor, onS const changes = findChangedNodes(prevState.doc, view.state.doc); if (changes.length > 0) { - // An 'attrs' or 'replaced' change alters a block's own identity (e.g. a - // heading's level, or switching bullet_list <-> ordered_list) — the - // outline and other editorHtmlChange consumers need a full re-parse for - // that, not the in-place text sync a plain edit gets. - const identityChanged = changes.some((c) => c.type === 'attrs' || c.type === 'replaced'); + // Only an EDITABLE_TYPES node changing its own attrs/type (heading level, + // list-type swap) needs a full outline re-parse; the same change on e.g. an + // image's src does not, so it takes the in-place text sync instead. + const identityChanged = changes.some((c) => ( + (c.type === 'attrs' || c.type === 'replaced') && EDITABLE_TYPES.includes(changedNodeType(c)) + )); const commonEditable = identityChanged ? null : findCommonEditableAncestor(view, changes, prevState); diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index 47d4e29bf..941482637 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -118,11 +118,9 @@ export class EwEditorDoc extends LitElement { view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); } - // proseIndex is already a raw doc position (no lookup table needed). Content it - // points at (text or an image) isn't necessarily a whole top-level node, so use - // TextSelection.near — it resolves to the nearest valid selection without throwing. - // An image is the exception: it's a real node, so select it the same way a block - // is (NodeSelection + broadcast) to get layout-view parity via the quick-edit port. + // proseIndex may point mid-node, so TextSelection.near resolves to the nearest valid + // selection without throwing. An image is a real node: select it as a NodeSelection and + // broadcast, matching how blocks sync to the layout view. _scrollDocToProseIndex(proseIndex, kind) { if (proseIndex == null || proseIndex < 0) return; const { view } = this._proseContext ?? {}; diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 0afe30270..81d29d8ec 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -41,6 +41,7 @@ function contentChildLabel(child) { case 'list': return child.ordered ? 'Numbered list' : 'Bullet list'; case 'image': return 'Image'; case 'code': return 'Code block'; + case 'quote': return 'Blockquote'; default: return 'Paragraph'; } } @@ -60,8 +61,6 @@ function sectionsEqual(a, b) { return a.every((sec, i) => { const other = b[i]; return sec.sectionIndex === other.sectionIndex - && sec.blocks.length === other.blocks.length - && sec.blocks.every((blk, j) => blk.name === other.blocks[j].name) && sec.items.length === other.items.length && sec.items.every((item, j) => itemsEqual(item, other.items[j])); }); diff --git a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js index 5eb53869c..5e8f496fb 100644 --- a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js +++ b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js @@ -197,10 +197,22 @@ describe('parseSections', () => { expect(children[1].ordered).to.equal(false); }); - it('classifies a

    -wrapped image as image, not paragraph (prose2aem only unwraps the

    when the image is the section\'s sole child)', () => { + it('classifies a top-level

    as a quote, not an image', () => { + const html = `
    +
    Some wisdom
    +
    `; + const [section] = parseSections(html); + const [{ children }] = section.items; + expect(children.map((c) => c.kind)).to.deep.equal(['quote']); + expect(children[0].proseIndex).to.equal(1); + }); + + it('classifies a

    -wrapped image as image, not paragraph (prose2aem only unwraps the

    when the image is the section\'s sole child), and reads the nested image index rather than the

    \'s own', () => { + // getInstrumentedHTML stamps data-prose-index on every outermost

    , including + // one that only wraps a — so a realistic fixture must include it too. const html = `

    Title

    -

    +

    Caption text

    `; const [section] = parseSections(html); diff --git a/test/unit/blocks/canvas/editor-utils/prose-diff.test.js b/test/unit/blocks/canvas/editor-utils/prose-diff.test.js index 464431f91..e334cb5bd 100644 --- a/test/unit/blocks/canvas/editor-utils/prose-diff.test.js +++ b/test/unit/blocks/canvas/editor-utils/prose-diff.test.js @@ -96,3 +96,31 @@ describe('createTrackingPlugin — block identity changes', () => { expect(counts()).to.deep.equal({ rerenderCalls: 0, getEditorCalls: 1 }); }); }); + +function setupImage(src) { + let rerenderCalls = 0; + let getEditorCalls = 0; + const plugin = createTrackingPlugin( + () => { rerenderCalls += 1; }, + undefined, + () => { getEditorCalls += 1; }, + undefined, + ); + const para = schema.nodes.paragraph.create(null, schema.nodes.image.create({ src })); + const doc = schema.nodes.doc.create(null, para); + const prevState = EditorState.create({ schema, doc, plugins: [plugin] }); + return { plugin, prevState, counts: () => ({ rerenderCalls, getEditorCalls }) }; +} + +describe('createTrackingPlugin — attrs changes outside EDITABLE_TYPES', () => { + it('changing an image\'s attrs (e.g. src) takes the lightweight getEditor path, not a full rerenderPage', () => { + const { plugin, prevState, counts } = setupImage('/a.png'); + const imagePos = 1; + const tr = prevState.tr.setNodeMarkup(imagePos, undefined, { src: '/b.png' }); + const nextState = prevState.apply(tr); + + plugin.spec.view().update({ state: nextState }, prevState); + + expect(counts()).to.deep.equal({ rerenderCalls: 0, getEditorCalls: 1 }); + }); +}); diff --git a/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js b/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js new file mode 100644 index 000000000..5dd17c325 --- /dev/null +++ b/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js @@ -0,0 +1,112 @@ +/* eslint-disable no-underscore-dangle */ +import { expect } from '@esm-bundle/chai'; +import { NodeSelection, TextSelection } from 'da-y-wrapper'; +import { setNx } from '../../../../../scripts/utils.js'; +import { createTestEditor, destroyEditor } from '../../edit/prose/test-helpers.js'; + +setNx('/test/fixtures/nx', { hostname: 'example.com' }); + +before(async () => { + await import('../../../../../blocks/canvas/ew-editor-doc/ew-editor-doc.js'); +}); + +// Wraps view.dispatch so tests can assert whether the guarded early-returns in +// _scrollDocToProseIndex actually skip dispatching, while still letting dispatched +// transactions apply so the resulting selection can be inspected. +function spyDispatch(view) { + const calls = []; + const original = view.dispatch.bind(view); + view.dispatch = (tr) => { + calls.push(tr); + original(tr); + }; + return calls; +} + +// Replaces the default single-paragraph doc with a text paragraph followed by a +// paragraph wrapping an image, mirroring how a real page mixes prose and images. +function buildDoc(view) { + const { schema } = view.state; + const textPara = schema.nodes.paragraph.create(null, schema.text('hello world')); + const imagePara = schema.nodes.paragraph.create(null, schema.nodes.image.create({ src: '/x.png' })); + const { content } = schema.nodes.doc.create(null, [textPara, imagePara]); + view.dispatch(view.state.tr.replaceWith(0, view.state.doc.content.size, content)); + + let imagePos = -1; + view.state.doc.descendants((node, pos) => { + if (node.type.name === 'image') imagePos = pos; + }); + return { imagePos }; +} + +describe('EwEditorDoc — _scrollDocToProseIndex', () => { + let editor; + let el; + let imagePos; + + beforeEach(async () => { + editor = await createTestEditor(); + ({ imagePos } = buildDoc(editor.view)); + el = document.createElement('ew-editor-doc'); + }); + + afterEach(() => { + destroyEditor(editor); + }); + + it('selects the image node with a NodeSelection and broadcasts when kind is image and the node at proseIndex is an image', () => { + const dispatchCalls = spyDispatch(editor.view); + const broadcastCalls = []; + el._broadcastSelectedNode = (...args) => broadcastCalls.push(args); + el._proseContext = { view: editor.view }; + + el._scrollDocToProseIndex(imagePos, 'image'); + + expect(dispatchCalls).to.have.lengthOf(1); + expect(editor.view.state.selection).to.be.instanceOf(NodeSelection); + expect(editor.view.state.selection.from).to.equal(imagePos); + expect(broadcastCalls).to.deep.equal([[true]]); + }); + + it('creates a TextSelection near proseIndex for a non-image kind and does not broadcast', () => { + const dispatchCalls = spyDispatch(editor.view); + const broadcastCalls = []; + el._broadcastSelectedNode = (...args) => broadcastCalls.push(args); + el._proseContext = { view: editor.view }; + + el._scrollDocToProseIndex(3, 'paragraph'); + + expect(dispatchCalls).to.have.lengthOf(1); + expect(editor.view.state.selection).to.be.instanceOf(TextSelection); + expect(broadcastCalls).to.deep.equal([]); + }); + + describe('guards', () => { + it('does nothing when proseIndex is null', () => { + const dispatchCalls = spyDispatch(editor.view); + el._proseContext = { view: editor.view }; + + el._scrollDocToProseIndex(null, 'text'); + + expect(dispatchCalls).to.have.lengthOf(0); + }); + + it('does nothing when proseIndex is negative', () => { + const dispatchCalls = spyDispatch(editor.view); + el._proseContext = { view: editor.view }; + + el._scrollDocToProseIndex(-1, 'text'); + + expect(dispatchCalls).to.have.lengthOf(0); + }); + + it('does nothing when proseIndex exceeds the document size', () => { + const dispatchCalls = spyDispatch(editor.view); + el._proseContext = { view: editor.view }; + + el._scrollDocToProseIndex(editor.view.state.doc.content.size + 10, 'text'); + + expect(dispatchCalls).to.have.lengthOf(0); + }); + }); +}); From 834b7186745797ef1986ce88c46c81ae1ed0fb01 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Thu, 23 Jul 2026 10:08:14 -0700 Subject: [PATCH 05/21] feat(canvas): arrow-key expand/collapse for outline default-content groups ArrowRight expands a collapsed "Default content" group header, ArrowLeft collapses an expanded one, by re-firing the header's existing click handler so no extra proseIndex plumbing is needed. Falls through to the existing treeKeydown nav otherwise. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/ew-page-outline/ew-page-outline.js | 13 ++++++++++++- .../ew-page-outline/ew-page-outline.test.js | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 81d29d8ec..728097f16 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -236,7 +236,18 @@ class EwPageOutline extends LitElement { } }; - _onTreeKeydown = (e) => treeKeydown(e, this.shadowRoot); + _onTreeKeydown = (e) => { + const item = this.shadowRoot.activeElement; + if (item?.matches('.content-item[aria-expanded]')) { + const expanded = item.getAttribute('aria-expanded') === 'true'; + if ((e.key === 'ArrowRight' && !expanded) || (e.key === 'ArrowLeft' && expanded)) { + e.preventDefault(); + item.click(); + return; + } + } + treeKeydown(e, this.shadowRoot); + }; async _openAddBlockModal(e, sectionIndex) { e.stopPropagation(); diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index 66053283d..283c447b2 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -104,4 +104,19 @@ describe('ew-page-outline — expandable default content', () => { expect(received).to.deep.equal({ proseIndex: 9, kind: 'image' }); }); + + it('expands and collapses the focused group header with ArrowRight/ArrowLeft', async () => { + const header = el.shadowRoot.querySelector('.content-item'); + header.focus(); + + header.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + await el.updateComplete; + expect(header.getAttribute('aria-expanded')).to.equal('true'); + expect(el.shadowRoot.querySelector('.content-children')).to.exist; + + header.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowLeft', bubbles: true })); + await el.updateComplete; + expect(header.getAttribute('aria-expanded')).to.equal('false'); + expect(el.shadowRoot.querySelector('.content-children')).to.be.null; + }); }); From 0ec880b788299af1c1f6721e85b10dcb82d49c28 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Fri, 24 Jul 2026 09:36:23 -0700 Subject: [PATCH 06/21] feat(canvas): drag-reorder and delete for default-content items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lets default-content children (paragraph/heading/list/image/code/quote) be dragged anywhere in the doc — within a group, across groups in a section, or into a different section entirely (including one with no default content yet) — and deleted individually, matching the existing block/section drag+delete UI. Cross-type moves land as siblings only (never merged into a block). blocks.js gains getContentItemRange/deleteContentItem/moveContentItem and getSectionEndOffset; moveBlock's delete+insert math is extracted into a shared spliceNode helper (no behavior change, now covered by regression tests since it previously had none). Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/blocks.js | 87 ++++++- .../canvas/ew-page-outline/ew-page-outline.js | 76 +++++- .../blocks/canvas/editor-utils/blocks.test.js | 239 ++++++++++++++++++ .../ew-page-outline/ew-page-outline.test.js | 115 +++++++++ test/unit/blocks/canvas/test-helpers.js | 22 ++ 5 files changed, 522 insertions(+), 17 deletions(-) create mode 100644 test/unit/blocks/canvas/editor-utils/blocks.test.js create mode 100644 test/unit/blocks/canvas/test-helpers.js diff --git a/blocks/canvas/editor-utils/blocks.js b/blocks/canvas/editor-utils/blocks.js index 40f228505..ff25e2f40 100644 --- a/blocks/canvas/editor-utils/blocks.js +++ b/blocks/canvas/editor-utils/blocks.js @@ -44,6 +44,17 @@ export function getActiveBlockIndex(view) { return -1; } +// Shared by every single-node move; adjusts insertPos for the shift the delete causes. +function spliceNode(view, from, insertPos) { + const adjustedInsertPos = insertPos > from.pos ? insertPos - from.size : insertPos; + if (adjustedInsertPos === from.pos) return; + view.dispatch( + view.state.tr + .delete(from.pos, from.pos + from.size) + .insert(adjustedInsertPos, from.node), + ); +} + export function moveBlock(view, fromIndex, toIndex, dropPosition) { if (!view) return; if (isSamePosition(fromIndex, toIndex, dropPosition)) return; @@ -60,20 +71,14 @@ export function moveBlock(view, fromIndex, toIndex, dropPosition) { if (!fromBlockNode || !toBlockNode) return; - const fromBlockSize = fromBlockNode.nodeSize; - const toBlockSize = toBlockNode.nodeSize; - const insertPos = dropPosition === 'before' ? toBlockPos - : toBlockPos + toBlockSize; - const adjustedInsertPos = insertPos > fromBlockPos - ? insertPos - fromBlockSize - : insertPos; + : toBlockPos + toBlockNode.nodeSize; - view.dispatch( - view.state.tr - .delete(fromBlockPos, fromBlockPos + fromBlockSize) - .insert(adjustedInsertPos, fromBlockNode), + spliceNode( + view, + { pos: fromBlockPos, size: fromBlockNode.nodeSize, node: fromBlockNode }, + insertPos, ); } @@ -87,6 +92,21 @@ export function deleteBlock(view, blockIndex) { view.dispatch(view.state.tr.delete(pos, pos + node.nodeSize)); } +// image proseIndex points at the nested ; its text-less

    wrapper always starts +// one position earlier (getDefaultContentKind guarantees the

    wraps only that image). +export function getContentItemRange(doc, child) { + const pos = child.kind === 'image' ? child.proseIndex - 1 : child.proseIndex; + const node = doc.nodeAt(pos); + return node ? { pos, size: node.nodeSize, node } : null; +} + +export function deleteContentItem(view, child) { + if (!view) return; + const range = getContentItemRange(view.state.doc, child); + if (!range) return; + view.dispatch(view.state.tr.delete(range.pos, range.pos + range.size)); +} + function getSectionStartOffset(view, sectionIndex) { const { doc, schema } = view.state; if (sectionIndex === 0) return 0; @@ -167,3 +187,48 @@ export function moveSection(view, fromSectionIndex, toSectionIndex, dropPosition view.dispatch(view.state.tr.replaceWith(0, doc.content.size, newNodes)); } + +// Counterpart to getSectionStartOffset — the hr position bounding the previous section. +function getSectionEndOffset(view, sectionIndex) { + if (sectionIndex === 0) return 0; + const { doc, schema } = view.state; + let hrCount = 0; + let result = 0; + doc.forEach((node, offset) => { + if (node.type === schema.nodes.horizontal_rule) { + hrCount += 1; + if (hrCount === sectionIndex) result = offset; + } + }); + return result; +} + +export function moveContentItem(view, fromChild, target, dropPosition) { + if (!view) return; + const { doc } = view.state; + const from = getContentItemRange(doc, fromChild); + if (!from) return; + + let insertPos; + if (target.type === 'content') { + const to = getContentItemRange(doc, target.child); + if (!to || to.pos === from.pos) return; + insertPos = dropPosition === 'before' ? to.pos : to.pos + to.size; + } else if (target.type === 'block') { + const positions = getBlockPositions(view); + if (target.blockIndex >= positions.length) return; + const toPos = positions[target.blockIndex]; + const toNode = doc.nodeAt(toPos); + if (!toNode) return; + insertPos = dropPosition === 'before' ? toPos : toPos + toNode.nodeSize; + } else if (target.type === 'section') { + // before the header = last item of the previous section, after = first of this one + insertPos = dropPosition === 'before' + ? getSectionEndOffset(view, target.sectionIndex) + : getSectionStartOffset(view, target.sectionIndex); + } else { + return; + } + + spliceNode(view, from, insertPos); +} diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 728097f16..7ab37ae5e 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -5,9 +5,11 @@ import { editorHtmlChange, editorSelectChange, editorProseSelectChange, parseSec import { getExtensionsBridge } from '../editor-utils/extensions-bridge.js'; import { deleteBlock, + deleteContentItem, deleteSection, insertBlockAtSectionStart, moveBlock, + moveContentItem, moveSection, } from '../editor-utils/blocks.js'; import { fetchExtensions } from '../ew-panel-extensions/helpers.js'; @@ -23,6 +25,7 @@ const style = await loadStyle(import.meta.url); const OUTLINE_TYPES = { SECTION: 'section', BLOCK: 'block', + CONTENT: 'content', }; const DROP_POSITIONS = { @@ -174,11 +177,12 @@ class EwPageOutline extends LitElement { } _onSectionDragOver(e, sec) { + const type = this._dragging?.type; const rect = e.currentTarget.getBoundingClientRect(); const dropPosition = e.clientY < rect.top + rect.height / 2 ? DROP_POSITIONS.BEFORE : DROP_POSITIONS.AFTER; - if (this._dragging?.type === OUTLINE_TYPES.SECTION) { + if (type === OUTLINE_TYPES.SECTION) { if (this._dragging.index === sec.sectionIndex) return; e.preventDefault(); @@ -187,6 +191,17 @@ class EwPageOutline extends LitElement { : e.currentTarget; this._setDropIndicator(el, { sectionIndex: sec.sectionIndex, dropPosition }); + } else if (type === OUTLINE_TYPES.CONTENT) { + // Bubbles here from anywhere unclaimed in the section; on the header itself we're + // before/after-aware, elsewhere (e.g. an empty section) we default to "first item". + e.preventDefault(); + const headerEl = e.currentTarget.querySelector('[data-section-header]'); + const onHeader = headerEl?.contains(e.target); + const contentDropPosition = onHeader ? dropPosition : DROP_POSITIONS.AFTER; + this._setDropIndicator( + headerEl, + { sectionIndex: sec.sectionIndex, dropPosition: contentDropPosition }, + ); } else { if (!sec.blocks.length) return; if (sec.blocks.some((b) => b.blockIndex === this._dragging?.index)) return; @@ -200,7 +215,9 @@ class EwPageOutline extends LitElement { } _onBlockDragOver(e, blockIndex) { - if (this._dragging?.type !== OUTLINE_TYPES.BLOCK || this._dragging.index === blockIndex) return; + const type = this._dragging?.type; + if (![OUTLINE_TYPES.BLOCK, OUTLINE_TYPES.CONTENT].includes(type)) return; + if (type === OUTLINE_TYPES.BLOCK && this._dragging.index === blockIndex) return; e.preventDefault(); e.stopPropagation(); const rect = e.currentTarget.getBoundingClientRect(); @@ -209,6 +226,30 @@ class EwPageOutline extends LitElement { this._setDropIndicator(e.currentTarget, { blockIndex, dropPosition }); } + _onContentDragOver(e, child) { + if (this._dragging?.type !== OUTLINE_TYPES.CONTENT) return; + if (this._dragging.index.proseIndex === child.proseIndex) return; + e.preventDefault(); + e.stopPropagation(); + const rect = e.currentTarget.getBoundingClientRect(); + const dropPosition = e.clientY < rect.top + rect.height / 2 + ? DROP_POSITIONS.BEFORE : DROP_POSITIONS.AFTER; + this._setDropIndicator(e.currentTarget, { contentChild: child, dropPosition }); + } + + _onContentGroupDragOver(e, item) { + if (this._dragging?.type !== OUTLINE_TYPES.CONTENT) return; + e.preventDefault(); + e.stopPropagation(); + const rect = e.currentTarget.getBoundingClientRect(); + const dropPosition = e.clientY < rect.top + rect.height / 2 + ? DROP_POSITIONS.BEFORE : DROP_POSITIONS.AFTER; + const targetChild = dropPosition === DROP_POSITIONS.BEFORE + ? item.children[0] + : item.children[item.children.length - 1]; + this._setDropIndicator(e.currentTarget, { contentChild: targetChild, dropPosition }); + } + _onDrop = (e) => { e.preventDefault(); e.stopPropagation(); @@ -216,7 +257,15 @@ class EwPageOutline extends LitElement { this._clearDragState(); if (!_dropTarget || !_dragging) return; const { view } = getExtensionsBridge(); - if (_dropTarget.blockIndex != null) { + + if (_dragging.type === OUTLINE_TYPES.CONTENT) { + let target; + if (_dropTarget.contentChild) target = { type: 'content', child: _dropTarget.contentChild }; + else if (_dropTarget.blockIndex != null) target = { type: 'block', blockIndex: _dropTarget.blockIndex }; + else if (_dropTarget.sectionIndex != null) target = { type: 'section', sectionIndex: _dropTarget.sectionIndex }; + else return; + moveContentItem(view, _dragging.index, target, _dropTarget.dropPosition); + } else if (_dropTarget.blockIndex != null) { if (_dragging.type !== OUTLINE_TYPES.BLOCK) return; moveBlock(view, _dragging.index, _dropTarget.blockIndex, _dropTarget.dropPosition); } else if (_dropTarget.sectionIndex != null) { @@ -267,14 +316,18 @@ class EwPageOutline extends LitElement { if (!view) return; if (type === OUTLINE_TYPES.BLOCK) { deleteBlock(view, index); + } else if (type === OUTLINE_TYPES.CONTENT) { + deleteContentItem(view, index); } else { deleteSection(view, index); } } _renderDeleteButton(type, index) { - const label = type === OUTLINE_TYPES.SECTION - ? `Delete section ${index + 1}` : 'Delete block'; + let noun = 'block'; + if (type === OUTLINE_TYPES.SECTION) noun = `section ${index + 1}`; + else if (type === OUTLINE_TYPES.CONTENT) noun = contentChildLabel(index).toLowerCase(); + const label = `Delete ${noun}`; return html`

    ${expanded ? html`
      ${item.children.map((child) => html`
    • this._onDragStart(e, OUTLINE_TYPES.CONTENT, child)} + @dragover=${(e) => this._onContentDragOver(e, child)} + @drop=${this._onDrop} + @dragend=${this._onDragEnd} @click=${(e) => { e.stopPropagation(); this._selectProse(child.proseIndex, child.kind); }}> + ${this._renderDeleteButton(OUTLINE_TYPES.CONTENT, child)} +
    • `)}
    ` : nothing} `; diff --git a/test/unit/blocks/canvas/editor-utils/blocks.test.js b/test/unit/blocks/canvas/editor-utils/blocks.test.js new file mode 100644 index 000000000..718a72915 --- /dev/null +++ b/test/unit/blocks/canvas/editor-utils/blocks.test.js @@ -0,0 +1,239 @@ +import { expect } from '@esm-bundle/chai'; +import { + getContentItemRange, + deleteContentItem, + moveContentItem, + moveBlock, +} from '../../../../../blocks/canvas/editor-utils/blocks.js'; +import { makeView, posOf } from '../test-helpers.js'; + +function docTypes(doc) { + const types = []; + doc.forEach((n) => types.push(n.type.name)); + return types; +} + +function tableJSON(name, contentText = 'content') { + const cell = (text) => ({ + type: 'table_cell', + content: [{ type: 'paragraph', content: [{ type: 'text', text }] }], + }); + return { + type: 'table', + content: [ + { type: 'table_row', content: [cell(name)] }, + { type: 'table_row', content: [cell(contentText)] }, + ], + }; +} + +describe('getContentItemRange', () => { + it('resolves a non-image child to its own node range', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Para one' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'Para two' }] }, + ], + }); + const { doc } = view.state; + const pos = posOf(doc, (n) => n.textContent === 'Para one'); + const range = getContentItemRange(doc, { kind: 'paragraph', proseIndex: pos }); + expect(range.pos).to.equal(pos); + expect(range.size).to.equal(doc.nodeAt(pos).nodeSize); + }); + + it('resolves an image child to its wrapping

    , not just the inline image node', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Before' }] }, + { type: 'paragraph', content: [{ type: 'image', attrs: { src: 'x.png' } }] }, + ], + }); + const { doc } = view.state; + const wrapperPos = posOf(doc, (n) => n.type.name === 'paragraph' && n.textContent === ''); + const wrapper = doc.nodeAt(wrapperPos); + const imagePos = wrapperPos + 1; + + const range = getContentItemRange(doc, { kind: 'image', proseIndex: imagePos }); + expect(range.pos).to.equal(wrapperPos); + expect(range.size).to.equal(wrapper.nodeSize); + expect(range.node.type.name).to.equal('paragraph'); + }); +}); + +describe('deleteContentItem', () => { + it('removes a paragraph child entirely', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'Delete me' }] }, + ], + }); + const pos = posOf(view.state.doc, (n) => n.textContent === 'Delete me'); + deleteContentItem(view, { kind: 'paragraph', proseIndex: pos }); + + const texts = []; + view.state.doc.forEach((n) => texts.push(n.textContent)); + expect(texts).to.deep.equal(['Keep me']); + }); + + it('removes the whole wrapping

    for an image child — no orphaned empty paragraph', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, + { type: 'paragraph', content: [{ type: 'image', attrs: { src: 'x.png' } }] }, + ], + }); + const wrapperPos = posOf(view.state.doc, (n) => n.type.name === 'paragraph' && n.textContent === ''); + const imagePos = wrapperPos + 1; + deleteContentItem(view, { kind: 'image', proseIndex: imagePos }); + + expect(view.state.doc.childCount).to.equal(1); + expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); + }); +}); + +describe('moveContentItem', () => { + it('reorders a content child relative to another content child (content target)', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + const aPos = posOf(view.state.doc, (n) => n.textContent === 'A'); + const bPos = posOf(view.state.doc, (n) => n.textContent === 'B'); + + moveContentItem( + view, + { kind: 'paragraph', proseIndex: aPos }, + { type: 'content', child: { kind: 'paragraph', proseIndex: bPos } }, + 'after', + ); + + const texts = []; + view.state.doc.forEach((n) => texts.push(n.textContent)); + expect(texts).to.deep.equal(['B', 'A']); + }); + + it('moves a content child before a block without merging into it (block target)', () => { + const view = makeView({ + type: 'doc', + content: [ + tableJSON('hero'), + { type: 'paragraph', content: [{ type: 'text', text: 'Loose para' }] }, + ], + }); + const paraPos = posOf(view.state.doc, (n) => n.type.name === 'paragraph'); + + moveContentItem( + view, + { kind: 'paragraph', proseIndex: paraPos }, + { type: 'block', blockIndex: 0 }, + 'before', + ); + + expect(docTypes(view.state.doc)).to.deep.equal(['paragraph', 'table']); + expect(view.state.doc.firstChild.textContent).to.equal('Loose para'); + }); + + it('moves a content child into a section with no default content yet (section target)', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Move me' }] }, + { type: 'horizontal_rule' }, + tableJSON('hero'), + ], + }); + const paraPos = posOf(view.state.doc, (n) => n.type.name === 'paragraph'); + + moveContentItem( + view, + { kind: 'paragraph', proseIndex: paraPos }, + { type: 'section', sectionIndex: 1 }, + 'after', + ); + + expect(docTypes(view.state.doc)).to.deep.equal(['horizontal_rule', 'paragraph', 'table']); + }); + + it('lands right before a section header — last item of the previous section', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'horizontal_rule' }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + const bPos = posOf(view.state.doc, (n) => n.textContent === 'B'); + + moveContentItem( + view, + { kind: 'paragraph', proseIndex: bPos }, + { type: 'section', sectionIndex: 1 }, + 'before', + ); + + expect(docTypes(view.state.doc)).to.deep.equal(['paragraph', 'paragraph', 'horizontal_rule']); + expect(view.state.doc.child(1).textContent).to.equal('B'); + }); + + it('does not dispatch when the drop position is a no-op', () => { + const view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + const aPos = posOf(view.state.doc, (n) => n.textContent === 'A'); + const bPos = posOf(view.state.doc, (n) => n.textContent === 'B'); + const before = view.state; + + moveContentItem( + view, + { kind: 'paragraph', proseIndex: aPos }, + { type: 'content', child: { kind: 'paragraph', proseIndex: bPos } }, + 'before', + ); + + expect(view.state).to.equal(before); + }); +}); + +describe('moveBlock (regression coverage for the shared splice helper)', () => { + it('reorders two blocks', () => { + const view = makeView({ + type: 'doc', + content: [tableJSON('hero'), tableJSON('cards')], + }); + moveBlock(view, 0, 1, 'after'); + + const names = []; + view.state.doc.descendants((n) => { + if (n.type.name === 'table') names.push(n.firstChild.firstChild.textContent); + }); + expect(names).to.deep.equal(['cards', 'hero']); + }); + + it('drops a block before another one', () => { + const view = makeView({ + type: 'doc', + content: [tableJSON('hero'), tableJSON('cards'), tableJSON('columns')], + }); + moveBlock(view, 2, 0, 'before'); + + const names = []; + view.state.doc.descendants((n) => { + if (n.type.name === 'table') names.push(n.firstChild.firstChild.textContent); + }); + expect(names).to.deep.equal(['columns', 'hero', 'cards']); + }); +}); diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index 283c447b2..d0ef81c64 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -1,16 +1,25 @@ /* eslint-disable no-underscore-dangle */ import { expect } from '@esm-bundle/chai'; import { setNx } from '../../../../../scripts/utils.js'; +import { makeView, posOf } from '../test-helpers.js'; setNx('/test/fixtures/nx', { hostname: 'example.com' }); let editorProseSelectChange; +let getExtensionsBridge; before(async () => { await import('../../../../../blocks/canvas/ew-page-outline/ew-page-outline.js'); ({ editorProseSelectChange } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); + ({ getExtensionsBridge } = await import('../../../../../blocks/canvas/editor-utils/extensions-bridge.js')); }); +function docSeq(doc) { + const seq = []; + doc.forEach((n) => seq.push(n.type.name === 'horizontal_rule' ? 'hr' : n.textContent)); + return seq; +} + async function createOutline() { const el = document.createElement('ew-page-outline'); // _checkBlockLibrary fires once a hash with org/site is set — no-op it so this @@ -120,3 +129,109 @@ describe('ew-page-outline — expandable default content', () => { expect(el.shadowRoot.querySelector('.content-children')).to.be.null; }); }); + +describe('ew-page-outline — content drag & delete', () => { + let el; + let bridge; + + beforeEach(async () => { + el = await createOutline(); + bridge = getExtensionsBridge(); + }); + + afterEach(() => { + el.remove(); + bridge.view = null; + }); + + it('deletes a content child via its delete button', async () => { + bridge.view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'Delete me' }] }, + ], + }); + const deletePos = posOf(bridge.view.state.doc, (n) => n.textContent === 'Delete me'); + + el._sections = [{ + sectionIndex: 0, + blocks: [], + items: [contentGroupItem(deletePos, [ + { type: 'content', kind: 'paragraph', proseIndex: deletePos, innerText: 'Delete me' }, + ])], + }]; + await el.updateComplete; + el.shadowRoot.querySelector('.content-item').click(); + await el.updateComplete; + + el.shadowRoot.querySelector('.content-child .delete-btn').click(); + + expect(docSeq(bridge.view.state.doc)).to.deep.equal(['Keep me']); + }); + + it('reorders content children via drop onto another content child', () => { + bridge.view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + const aPos = posOf(bridge.view.state.doc, (n) => n.textContent === 'A'); + const bPos = posOf(bridge.view.state.doc, (n) => n.textContent === 'B'); + const childA = { kind: 'paragraph', proseIndex: aPos }; + const childB = { kind: 'paragraph', proseIndex: bPos }; + + el._dragging = { type: 'content', index: childA }; + el._dropTarget = { contentChild: childB, dropPosition: 'after' }; + el._onDrop({ preventDefault() {}, stopPropagation() {} }); + + expect(docSeq(bridge.view.state.doc)).to.deep.equal(['B', 'A']); + }); + + it('routes a content drop onto a section header through moveContentItem', () => { + bridge.view = makeView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Move me' }] }, + { type: 'horizontal_rule' }, + { type: 'paragraph', content: [{ type: 'text', text: 'Existing' }] }, + ], + }); + const movePos = posOf(bridge.view.state.doc, (n) => n.textContent === 'Move me'); + + el._dragging = { type: 'content', index: { kind: 'paragraph', proseIndex: movePos } }; + el._dropTarget = { sectionIndex: 1, dropPosition: 'after' }; + el._onDrop({ preventDefault() {}, stopPropagation() {} }); + + expect(docSeq(bridge.view.state.doc)).to.deep.equal(['hr', 'Move me', 'Existing']); + }); + + it('dropping on a group header before/after targets the first/last child', () => { + const item = { + proseIndex: 1, + children: [ + { kind: 'paragraph', proseIndex: 1, innerText: 'first' }, + { kind: 'paragraph', proseIndex: 5, innerText: 'last' }, + ], + }; + el._dragging = { type: 'content', index: { kind: 'paragraph', proseIndex: 99 } }; + + const rect = { top: 0, height: 20 }; + const fakeEvent = (clientY) => ({ + preventDefault() {}, + stopPropagation() {}, + currentTarget: { getBoundingClientRect: () => rect, dataset: {} }, + clientY, + }); + + el._onContentGroupDragOver(fakeEvent(5), item); + expect(el._dropTarget.contentChild).to.deep.equal(item.children[0]); + expect(el._dropTarget.dropPosition).to.equal('before'); + + el._onContentGroupDragOver(fakeEvent(15), item); + expect(el._dropTarget.contentChild).to.deep.equal(item.children[1]); + expect(el._dropTarget.dropPosition).to.equal('after'); + }); +}); diff --git a/test/unit/blocks/canvas/test-helpers.js b/test/unit/blocks/canvas/test-helpers.js new file mode 100644 index 000000000..b7d2834c4 --- /dev/null +++ b/test/unit/blocks/canvas/test-helpers.js @@ -0,0 +1,22 @@ +import { EditorState } from 'da-y-wrapper'; +import { getSchema } from 'da-parser'; + +const schema = getSchema(); + +export function makeView(json) { + const doc = schema.nodeFromJSON(json); + let state = EditorState.create({ schema, doc }); + return { + get state() { return state; }, + dispatch(tr) { state = state.apply(tr); }, + }; +} + +// avoids fragile hand-computed offsets once more than one node's size is involved +export function posOf(doc, match) { + let result; + doc.forEach((node, offset) => { + if (result === undefined && match(node)) result = offset; + }); + return result; +} From 605da1e6d30e84a24e5955abdf372550f48af907 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Fri, 24 Jul 2026 10:18:24 -0700 Subject: [PATCH 07/21] fix(canvas): correct default-content proseIndex resolution; let blocks/content interoperate as drop targets getContentItemRange assumed proseIndex already pointed at a content item's own node start. In reality getInstrumentedHTML stamps it at the node's content-start (view.posAtDOM(el, 0)), which only coincided with the node start for the image case already special-cased. Every other kind was broken: moving/deleting a paragraph, heading, or code block would only touch its inner text (orphaning an empty wrapper), a quote would drop two levels too deep, and a multi-item list would only ever move/delete its first item. Fixed generally via doc.resolve(proseIndex).before(1), which recovers the true top-level node regardless of kind or nesting depth. Also closes a gap where blocks and content items couldn't target each other when dragging: content rows/groups only accepted content-type drags, and a block dragged into a section with no blocks (including a wholly empty one) had nowhere to land. Adds moveBlockToContentItem and moveBlockToSection so blocks and content items are drop targets for each other in both directions. Supersedes PR #1166 (dropempt), which fixed only the empty-section case in isolation. Rewrote blocks.js/ew-page-outline test coverage to build child descriptors via a real EditorView + getInstrumentedHTML + parseSections (makeRealView in test-helpers.js) instead of hand-picked node positions, since that gap is exactly what let the original bug through untested. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/blocks.js | 49 ++- .../canvas/ew-page-outline/ew-page-outline.js | 39 ++- .../blocks/canvas/editor-utils/blocks.test.js | 280 ++++++++++++------ .../ew-page-outline/ew-page-outline.test.js | 101 +++++-- test/unit/blocks/canvas/test-helpers.js | 16 +- 5 files changed, 365 insertions(+), 120 deletions(-) diff --git a/blocks/canvas/editor-utils/blocks.js b/blocks/canvas/editor-utils/blocks.js index ff25e2f40..9dc291d9f 100644 --- a/blocks/canvas/editor-utils/blocks.js +++ b/blocks/canvas/editor-utils/blocks.js @@ -92,10 +92,13 @@ export function deleteBlock(view, blockIndex) { view.dispatch(view.state.tr.delete(pos, pos + node.nodeSize)); } -// image proseIndex points at the nested ; its text-less

    wrapper always starts -// one position earlier (getDefaultContentKind guarantees the

    wraps only that image). +// proseIndex is a position INSIDE the default-content node (getInstrumentedHTML stamps +// it at the node's content-start, e.g. where an image or a paragraph's text begins), not +// the node's own start — resolving to the depth-1 ancestor recovers the whole top-level +// node regardless of kind or how deeply proseIndex sits inside it (e.g. a blockquote's +// nested paragraph, or a list's first item). export function getContentItemRange(doc, child) { - const pos = child.kind === 'image' ? child.proseIndex - 1 : child.proseIndex; + const pos = doc.resolve(child.proseIndex).before(1); const node = doc.nodeAt(pos); return node ? { pos, size: node.nodeSize, node } : null; } @@ -232,3 +235,43 @@ export function moveContentItem(view, fromChild, target, dropPosition) { spliceNode(view, from, insertPos); } + +function getBlockRange(view, blockIndex) { + const { doc } = view.state; + const positions = getBlockPositions(view); + if (blockIndex >= positions.length) return null; + const pos = positions[blockIndex]; + const node = doc.nodeAt(pos); + return node ? { pos, size: node.nodeSize, node } : null; +} + +// Block dropped onto/near a default-content row — the reverse of moveContentItem's +// 'content' target. Block-to-block reordering stays on moveBlock; this only covers a +// block landing next to a content item. +export function moveBlockToContentItem(view, blockIndex, targetChild, dropPosition) { + if (!view) return; + const from = getBlockRange(view, blockIndex); + if (!from) return; + + const to = getContentItemRange(view.state.doc, targetChild); + if (!to) return; + const insertPos = dropPosition === 'before' ? to.pos : to.pos + to.size; + + spliceNode(view, from, insertPos); +} + +// Block dropped onto a section's header when that section has no blocks to anchor a +// drop indicator on — the reverse of moveContentItem's 'section' target. Whole-section +// reordering stays on moveSection; this only covers a lone block landing at the +// section boundary. +export function moveBlockToSection(view, blockIndex, sectionIndex, dropPosition) { + if (!view) return; + const from = getBlockRange(view, blockIndex); + if (!from) return; + + const insertPos = dropPosition === 'before' + ? getSectionEndOffset(view, sectionIndex) + : getSectionStartOffset(view, sectionIndex); + + spliceNode(view, from, insertPos); +} diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 7ab37ae5e..d0bbc515a 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -9,6 +9,8 @@ import { deleteSection, insertBlockAtSectionStart, moveBlock, + moveBlockToContentItem, + moveBlockToSection, moveContentItem, moveSection, } from '../editor-utils/blocks.js'; @@ -203,14 +205,25 @@ class EwPageOutline extends LitElement { { sectionIndex: sec.sectionIndex, dropPosition: contentDropPosition }, ); } else { - if (!sec.blocks.length) return; if (sec.blocks.some((b) => b.blockIndex === this._dragging?.index)) return; - const { blockIndex } = sec.blocks[sec.blocks.length - 1]; - e.preventDefault(); + if (sec.blocks.length) { + const { blockIndex } = sec.blocks[sec.blocks.length - 1]; + e.preventDefault(); + + const lastBlockEl = this.shadowRoot.querySelector(`[data-block-index="${blockIndex}"]`); + if (!lastBlockEl) return; + this._setDropIndicator(lastBlockEl, { blockIndex, dropPosition: DROP_POSITIONS.AFTER }); + return; + } - const lastBlockEl = this.shadowRoot.querySelector(`[data-block-index="${blockIndex}"]`); - if (!lastBlockEl) return; - this._setDropIndicator(lastBlockEl, { blockIndex, dropPosition: DROP_POSITIONS.AFTER }); + // No blocks in this section to anchor on (it may still have content, or be + // wholly empty) — fall back to the section boundary itself. + e.preventDefault(); + const headerEl = e.currentTarget.querySelector('[data-section-header]'); + this._setDropIndicator( + headerEl, + { sectionIndex: sec.sectionIndex, dropPosition: DROP_POSITIONS.AFTER }, + ); } } @@ -227,8 +240,11 @@ class EwPageOutline extends LitElement { } _onContentDragOver(e, child) { - if (this._dragging?.type !== OUTLINE_TYPES.CONTENT) return; - if (this._dragging.index.proseIndex === child.proseIndex) return; + const type = this._dragging?.type; + if (![OUTLINE_TYPES.CONTENT, OUTLINE_TYPES.BLOCK].includes(type)) return; + const isSameChild = type === OUTLINE_TYPES.CONTENT + && this._dragging.index.proseIndex === child.proseIndex; + if (isSameChild) return; e.preventDefault(); e.stopPropagation(); const rect = e.currentTarget.getBoundingClientRect(); @@ -238,7 +254,7 @@ class EwPageOutline extends LitElement { } _onContentGroupDragOver(e, item) { - if (this._dragging?.type !== OUTLINE_TYPES.CONTENT) return; + if (![OUTLINE_TYPES.CONTENT, OUTLINE_TYPES.BLOCK].includes(this._dragging?.type)) return; e.preventDefault(); e.stopPropagation(); const rect = e.currentTarget.getBoundingClientRect(); @@ -265,6 +281,11 @@ class EwPageOutline extends LitElement { else if (_dropTarget.sectionIndex != null) target = { type: 'section', sectionIndex: _dropTarget.sectionIndex }; else return; moveContentItem(view, _dragging.index, target, _dropTarget.dropPosition); + } else if (_dragging.type === OUTLINE_TYPES.BLOCK && _dropTarget.contentChild) { + const { contentChild, dropPosition } = _dropTarget; + moveBlockToContentItem(view, _dragging.index, contentChild, dropPosition); + } else if (_dragging.type === OUTLINE_TYPES.BLOCK && _dropTarget.sectionIndex != null) { + moveBlockToSection(view, _dragging.index, _dropTarget.sectionIndex, _dropTarget.dropPosition); } else if (_dropTarget.blockIndex != null) { if (_dragging.type !== OUTLINE_TYPES.BLOCK) return; moveBlock(view, _dragging.index, _dropTarget.blockIndex, _dropTarget.dropPosition); diff --git a/test/unit/blocks/canvas/editor-utils/blocks.test.js b/test/unit/blocks/canvas/editor-utils/blocks.test.js index 718a72915..d7c09b0b9 100644 --- a/test/unit/blocks/canvas/editor-utils/blocks.test.js +++ b/test/unit/blocks/canvas/editor-utils/blocks.test.js @@ -1,11 +1,33 @@ import { expect } from '@esm-bundle/chai'; +import { setNx } from '../../../../../scripts/utils.js'; import { getContentItemRange, deleteContentItem, moveContentItem, + moveBlockToContentItem, + moveBlockToSection, moveBlock, } from '../../../../../blocks/canvas/editor-utils/blocks.js'; -import { makeView, posOf } from '../test-helpers.js'; +import { makeView, makeRealView } from '../test-helpers.js'; + +setNx('/test/fixtures/nx', { hostname: 'example.com' }); + +let getInstrumentedHTML; +let parseSections; + +before(async () => { + ({ getInstrumentedHTML, parseSections } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); +}); + +// Builds the same `child` descriptors the outline actually drags/deletes — proseIndex +// comes from the real getInstrumentedHTML/parseSections pipeline, not a hand-picked +// node position, since that's what previously masked a whole class of bugs (proseIndex +// points inside a node's content, not at its own start — see getContentItemRange). +function childrenOf(view) { + const html = getInstrumentedHTML(view); + const sections = parseSections(html); + return sections.flatMap((section) => section.items.flatMap((item) => item.children ?? [])); +} function docTypes(doc) { const types = []; @@ -28,69 +50,99 @@ function tableJSON(name, contentText = 'content') { } describe('getContentItemRange', () => { - it('resolves a non-image child to its own node range', () => { - const view = makeView({ - type: 'doc', + const cases = [ + ['paragraph', { type: 'paragraph', content: [{ type: 'text', text: 'Para text' }] }, 'paragraph', 'Para text'], + ['heading', { type: 'heading', attrs: { level: 2 }, content: [{ type: 'text', text: 'Heading text' }] }, 'heading', 'Heading text'], + ['code block', { type: 'code_block', content: [{ type: 'text', text: 'const x = 1;' }] }, 'code_block', 'const x = 1;'], + ['quote', { type: 'blockquote', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Some wisdom' }] }] }, 'blockquote', 'Some wisdom'], + ['multi-item list', { + type: 'bullet_list', content: [ - { type: 'paragraph', content: [{ type: 'text', text: 'Para one' }] }, - { type: 'paragraph', content: [{ type: 'text', text: 'Para two' }] }, + { type: 'list_item', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'One' }] }] }, + { type: 'list_item', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Two' }] }] }, ], + }, 'bullet_list', 'OneTwo'], + ]; + + cases.forEach(([label, nodeJSON, expectedType, expectedText]) => { + it(`resolves a ${label} child to its whole node, not an inner fragment`, () => { + const view = makeRealView({ type: 'doc', content: [nodeJSON] }); + const [child] = childrenOf(view); + const range = getContentItemRange(view.state.doc, child); + expect(range.pos).to.equal(0); + expect(range.node.type.name).to.equal(expectedType); + expect(range.node.textContent).to.equal(expectedText); }); - const { doc } = view.state; - const pos = posOf(doc, (n) => n.textContent === 'Para one'); - const range = getContentItemRange(doc, { kind: 'paragraph', proseIndex: pos }); - expect(range.pos).to.equal(pos); - expect(range.size).to.equal(doc.nodeAt(pos).nodeSize); }); it('resolves an image child to its wrapping

    , not just the inline image node', () => { - const view = makeView({ + const view = makeRealView({ type: 'doc', - content: [ - { type: 'paragraph', content: [{ type: 'text', text: 'Before' }] }, - { type: 'paragraph', content: [{ type: 'image', attrs: { src: 'x.png' } }] }, - ], + content: [{ type: 'paragraph', content: [{ type: 'image', attrs: { src: 'x.png' } }] }], }); - const { doc } = view.state; - const wrapperPos = posOf(doc, (n) => n.type.name === 'paragraph' && n.textContent === ''); - const wrapper = doc.nodeAt(wrapperPos); - const imagePos = wrapperPos + 1; - - const range = getContentItemRange(doc, { kind: 'image', proseIndex: imagePos }); - expect(range.pos).to.equal(wrapperPos); - expect(range.size).to.equal(wrapper.nodeSize); + const [child] = childrenOf(view); + const range = getContentItemRange(view.state.doc, child); + expect(range.pos).to.equal(0); expect(range.node.type.name).to.equal('paragraph'); }); }); describe('deleteContentItem', () => { it('removes a paragraph child entirely', () => { - const view = makeView({ + const view = makeRealView({ type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, { type: 'paragraph', content: [{ type: 'text', text: 'Delete me' }] }, ], }); - const pos = posOf(view.state.doc, (n) => n.textContent === 'Delete me'); - deleteContentItem(view, { kind: 'paragraph', proseIndex: pos }); + const child = childrenOf(view).find((c) => c.innerText === 'Delete me'); + deleteContentItem(view, child); + + expect(view.state.doc.childCount).to.equal(1); + expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); + }); + + it('removes the whole code block, not just its text', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, + { type: 'code_block', content: [{ type: 'text', text: 'const x = 1;' }] }, + ], + }); + const child = childrenOf(view).find((c) => c.kind === 'code'); + deleteContentItem(view, child); - const texts = []; - view.state.doc.forEach((n) => texts.push(n.textContent)); - expect(texts).to.deep.equal(['Keep me']); + expect(view.state.doc.childCount).to.equal(1); + expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); + }); + + it('removes the whole blockquote, not just its inner paragraph text', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, + { type: 'blockquote', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Wisdom' }] }] }, + ], + }); + const child = childrenOf(view).find((c) => c.kind === 'quote'); + deleteContentItem(view, child); + + expect(view.state.doc.childCount).to.equal(1); + expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); }); it('removes the whole wrapping

    for an image child — no orphaned empty paragraph', () => { - const view = makeView({ + const view = makeRealView({ type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, { type: 'paragraph', content: [{ type: 'image', attrs: { src: 'x.png' } }] }, ], }); - const wrapperPos = posOf(view.state.doc, (n) => n.type.name === 'paragraph' && n.textContent === ''); - const imagePos = wrapperPos + 1; - deleteContentItem(view, { kind: 'image', proseIndex: imagePos }); + const child = childrenOf(view).find((c) => c.kind === 'image'); + deleteContentItem(view, child); expect(view.state.doc.childCount).to.equal(1); expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); @@ -98,73 +150,66 @@ describe('deleteContentItem', () => { }); describe('moveContentItem', () => { - it('reorders a content child relative to another content child (content target)', () => { - const view = makeView({ + it('moves a whole code block relative to another content child (content target)', () => { + const view = makeRealView({ type: 'doc', content: [ - { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'code_block', content: [{ type: 'text', text: 'const x = 1;' }] }, { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, ], }); - const aPos = posOf(view.state.doc, (n) => n.textContent === 'A'); - const bPos = posOf(view.state.doc, (n) => n.textContent === 'B'); + const [codeChild, paraChild] = childrenOf(view); - moveContentItem( - view, - { kind: 'paragraph', proseIndex: aPos }, - { type: 'content', child: { kind: 'paragraph', proseIndex: bPos } }, - 'after', - ); + moveContentItem(view, codeChild, { type: 'content', child: paraChild }, 'after'); - const texts = []; - view.state.doc.forEach((n) => texts.push(n.textContent)); - expect(texts).to.deep.equal(['B', 'A']); + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['paragraph', 'code_block']); + expect(doc.lastChild.textContent).to.equal('const x = 1;'); }); - it('moves a content child before a block without merging into it (block target)', () => { - const view = makeView({ + it('moves a whole quote before a block without merging into it (block target)', () => { + const view = makeRealView({ type: 'doc', content: [ tableJSON('hero'), - { type: 'paragraph', content: [{ type: 'text', text: 'Loose para' }] }, + { type: 'blockquote', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Wisdom' }] }] }, ], }); - const paraPos = posOf(view.state.doc, (n) => n.type.name === 'paragraph'); + const child = childrenOf(view).find((c) => c.kind === 'quote'); - moveContentItem( - view, - { kind: 'paragraph', proseIndex: paraPos }, - { type: 'block', blockIndex: 0 }, - 'before', - ); + moveContentItem(view, child, { type: 'block', blockIndex: 0 }, 'before'); - expect(docTypes(view.state.doc)).to.deep.equal(['paragraph', 'table']); - expect(view.state.doc.firstChild.textContent).to.equal('Loose para'); + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['blockquote', 'table']); + expect(doc.firstChild.textContent).to.equal('Wisdom'); }); - it('moves a content child into a section with no default content yet (section target)', () => { - const view = makeView({ + it('moves a whole multi-item list into a section with no default content yet (section target)', () => { + const view = makeRealView({ type: 'doc', content: [ - { type: 'paragraph', content: [{ type: 'text', text: 'Move me' }] }, + { + type: 'bullet_list', + content: [ + { type: 'list_item', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'One' }] }] }, + { type: 'list_item', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Two' }] }] }, + ], + }, { type: 'horizontal_rule' }, tableJSON('hero'), ], }); - const paraPos = posOf(view.state.doc, (n) => n.type.name === 'paragraph'); + const child = childrenOf(view).find((c) => c.kind === 'list'); - moveContentItem( - view, - { kind: 'paragraph', proseIndex: paraPos }, - { type: 'section', sectionIndex: 1 }, - 'after', - ); + moveContentItem(view, child, { type: 'section', sectionIndex: 1 }, 'after'); - expect(docTypes(view.state.doc)).to.deep.equal(['horizontal_rule', 'paragraph', 'table']); + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['horizontal_rule', 'bullet_list', 'table']); + expect(doc.child(1).textContent).to.equal('OneTwo'); }); it('lands right before a section header — last item of the previous section', () => { - const view = makeView({ + const view = makeRealView({ type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, @@ -172,42 +217,97 @@ describe('moveContentItem', () => { { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, ], }); - const bPos = posOf(view.state.doc, (n) => n.textContent === 'B'); + const child = childrenOf(view).find((c) => c.innerText === 'B'); - moveContentItem( - view, - { kind: 'paragraph', proseIndex: bPos }, - { type: 'section', sectionIndex: 1 }, - 'before', - ); + moveContentItem(view, child, { type: 'section', sectionIndex: 1 }, 'before'); - expect(docTypes(view.state.doc)).to.deep.equal(['paragraph', 'paragraph', 'horizontal_rule']); - expect(view.state.doc.child(1).textContent).to.equal('B'); + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['paragraph', 'paragraph', 'horizontal_rule']); + expect(doc.child(1).textContent).to.equal('B'); }); it('does not dispatch when the drop position is a no-op', () => { - const view = makeView({ + const view = makeRealView({ type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, ], }); - const aPos = posOf(view.state.doc, (n) => n.textContent === 'A'); - const bPos = posOf(view.state.doc, (n) => n.textContent === 'B'); + const [a, b] = childrenOf(view); const before = view.state; - moveContentItem( - view, - { kind: 'paragraph', proseIndex: aPos }, - { type: 'content', child: { kind: 'paragraph', proseIndex: bPos } }, - 'before', - ); + moveContentItem(view, a, { type: 'content', child: b }, 'before'); expect(view.state).to.equal(before); }); }); +describe('moveBlockToContentItem', () => { + it('moves a block before a content child, landing as a sibling in doc order', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Loose para' }] }, + tableJSON('hero'), + ], + }); + const child = childrenOf(view).find((c) => c.innerText === 'Loose para'); + + moveBlockToContentItem(view, 0, child, 'before'); + + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['table', 'paragraph']); + expect(doc.firstChild.firstChild.firstChild.textContent).to.equal('hero'); + }); + + it('moves a block after a content child', () => { + const view = makeRealView({ + type: 'doc', + content: [ + tableJSON('hero'), + { type: 'paragraph', content: [{ type: 'text', text: 'Loose para' }] }, + ], + }); + const child = childrenOf(view).find((c) => c.innerText === 'Loose para'); + + moveBlockToContentItem(view, 0, child, 'after'); + + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['paragraph', 'table']); + }); +}); + +describe('moveBlockToSection', () => { + it('moves a block into a section that has no blocks (including a wholly empty one)', () => { + const view = makeRealView({ + type: 'doc', + content: [{ type: 'horizontal_rule' }, tableJSON('hero')], + }); + + moveBlockToSection(view, 0, 0, 'after'); + + expect(docTypes(view.state.doc)).to.deep.equal(['table', 'horizontal_rule']); + }); + + it('lands right before a section header — last item of the previous section', () => { + const view = makeRealView({ + type: 'doc', + content: [tableJSON('hero'), { type: 'horizontal_rule' }, tableJSON('cards')], + }); + + moveBlockToSection(view, 1, 1, 'before'); + + const { doc } = view.state; + expect(docTypes(doc)).to.deep.equal(['table', 'table', 'horizontal_rule']); + const names = []; + doc.descendants((n) => { + if (n.type.name === 'table') names.push(n.firstChild.firstChild.textContent); + }); + expect(names).to.deep.equal(['hero', 'cards']); + }); +}); + describe('moveBlock (regression coverage for the shared splice helper)', () => { it('reorders two blocks', () => { const view = makeView({ diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index d0ef81c64..503836d0c 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -1,19 +1,30 @@ /* eslint-disable no-underscore-dangle */ import { expect } from '@esm-bundle/chai'; import { setNx } from '../../../../../scripts/utils.js'; -import { makeView, posOf } from '../test-helpers.js'; +import { makeRealView } from '../test-helpers.js'; setNx('/test/fixtures/nx', { hostname: 'example.com' }); let editorProseSelectChange; let getExtensionsBridge; +let getInstrumentedHTML; +let parseSections; before(async () => { await import('../../../../../blocks/canvas/ew-page-outline/ew-page-outline.js'); - ({ editorProseSelectChange } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); + ({ editorProseSelectChange, getInstrumentedHTML, parseSections } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); ({ getExtensionsBridge } = await import('../../../../../blocks/canvas/editor-utils/extensions-bridge.js')); }); +// Builds the same `child` descriptors the outline actually drags/deletes — proseIndex +// comes from the real getInstrumentedHTML/parseSections pipeline, not a hand-picked +// node position (proseIndex points inside a node's content, not at its own start). +function childrenOf(view) { + const html = getInstrumentedHTML(view); + const sections = parseSections(html); + return sections.flatMap((section) => section.items.flatMap((item) => item.children ?? [])); +} + function docSeq(doc) { const seq = []; doc.forEach((n) => seq.push(n.type.name === 'horizontal_rule' ? 'hr' : n.textContent)); @@ -145,21 +156,19 @@ describe('ew-page-outline — content drag & delete', () => { }); it('deletes a content child via its delete button', async () => { - bridge.view = makeView({ + bridge.view = makeRealView({ type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'Keep me' }] }, - { type: 'paragraph', content: [{ type: 'text', text: 'Delete me' }] }, + { type: 'code_block', content: [{ type: 'text', text: 'const x = 1;' }] }, ], }); - const deletePos = posOf(bridge.view.state.doc, (n) => n.textContent === 'Delete me'); + const child = childrenOf(bridge.view).find((c) => c.kind === 'code'); el._sections = [{ sectionIndex: 0, blocks: [], - items: [contentGroupItem(deletePos, [ - { type: 'content', kind: 'paragraph', proseIndex: deletePos, innerText: 'Delete me' }, - ])], + items: [contentGroupItem(child.proseIndex, [child])], }]; await el.updateComplete; el.shadowRoot.querySelector('.content-item').click(); @@ -171,17 +180,14 @@ describe('ew-page-outline — content drag & delete', () => { }); it('reorders content children via drop onto another content child', () => { - bridge.view = makeView({ + bridge.view = makeRealView({ type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, ], }); - const aPos = posOf(bridge.view.state.doc, (n) => n.textContent === 'A'); - const bPos = posOf(bridge.view.state.doc, (n) => n.textContent === 'B'); - const childA = { kind: 'paragraph', proseIndex: aPos }; - const childB = { kind: 'paragraph', proseIndex: bPos }; + const [childA, childB] = childrenOf(bridge.view); el._dragging = { type: 'content', index: childA }; el._dropTarget = { contentChild: childB, dropPosition: 'after' }; @@ -191,23 +197,84 @@ describe('ew-page-outline — content drag & delete', () => { }); it('routes a content drop onto a section header through moveContentItem', () => { - bridge.view = makeView({ + bridge.view = makeRealView({ type: 'doc', content: [ - { type: 'paragraph', content: [{ type: 'text', text: 'Move me' }] }, + { type: 'blockquote', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Move me' }] }] }, { type: 'horizontal_rule' }, { type: 'paragraph', content: [{ type: 'text', text: 'Existing' }] }, ], }); - const movePos = posOf(bridge.view.state.doc, (n) => n.textContent === 'Move me'); + const child = childrenOf(bridge.view).find((c) => c.kind === 'quote'); - el._dragging = { type: 'content', index: { kind: 'paragraph', proseIndex: movePos } }; + el._dragging = { type: 'content', index: child }; el._dropTarget = { sectionIndex: 1, dropPosition: 'after' }; el._onDrop({ preventDefault() {}, stopPropagation() {} }); expect(docSeq(bridge.view.state.doc)).to.deep.equal(['hr', 'Move me', 'Existing']); }); + it('routes a block dropped onto a content child through moveBlockToContentItem', () => { + bridge.view = makeRealView({ + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [{ type: 'table_cell', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hero' }] }] }], + }, + ], + }, + { type: 'paragraph', content: [{ type: 'text', text: 'Loose para' }] }, + ], + }); + const child = childrenOf(bridge.view).find((c) => c.innerText === 'Loose para'); + + el._dragging = { type: 'block', index: 0 }; + el._dropTarget = { contentChild: child, dropPosition: 'after' }; + el._onDrop({ preventDefault() {}, stopPropagation() {} }); + + expect(docSeq(bridge.view.state.doc)).to.deep.equal(['Loose para', 'hero']); + }); + + it('routes a block dropped onto an empty section through moveBlockToSection', () => { + const tableNode = (name) => ({ + type: 'table', + content: [{ + type: 'table_row', + content: [{ type: 'table_cell', content: [{ type: 'paragraph', content: [{ type: 'text', text: name }] }] }], + }], + }); + bridge.view = makeRealView({ + type: 'doc', + content: [{ type: 'horizontal_rule' }, tableNode('hero')], + }); + + el._dragging = { type: 'block', index: 0 }; + el._dropTarget = { sectionIndex: 0, dropPosition: 'after' }; + el._onDrop({ preventDefault() {}, stopPropagation() {} }); + + expect(docSeq(bridge.view.state.doc)).to.deep.equal(['hero', 'hr']); + }); + + it('accepts a block drag over a content child/group (sets a drop indicator, not just content drags)', () => { + const child = { kind: 'paragraph', proseIndex: 1 }; + el._dragging = { type: 'block', index: 0 }; + + const rect = { top: 0, height: 20 }; + const fakeEvent = (clientY) => ({ + preventDefault() {}, + stopPropagation() {}, + currentTarget: { getBoundingClientRect: () => rect, dataset: {} }, + clientY, + }); + + el._onContentDragOver(fakeEvent(15), child); + expect(el._dropTarget).to.deep.equal({ contentChild: child, dropPosition: 'after' }); + }); + it('dropping on a group header before/after targets the first/last child', () => { const item = { proseIndex: 1, diff --git a/test/unit/blocks/canvas/test-helpers.js b/test/unit/blocks/canvas/test-helpers.js index b7d2834c4..eeb098dd6 100644 --- a/test/unit/blocks/canvas/test-helpers.js +++ b/test/unit/blocks/canvas/test-helpers.js @@ -1,4 +1,4 @@ -import { EditorState } from 'da-y-wrapper'; +import { EditorState, EditorView } from 'da-y-wrapper'; import { getSchema } from 'da-parser'; const schema = getSchema(); @@ -12,6 +12,20 @@ export function makeView(json) { }; } +// A real, mounted EditorView — needed wherever a test goes through getInstrumentedHTML, +// since that relies on view.posAtDOM/view.dom, which a fake view can't provide. +export function makeRealView(json) { + const doc = schema.nodeFromJSON(json); + const state = EditorState.create({ schema, doc }); + const dom = document.createElement('div'); + document.body.appendChild(dom); + const view = new EditorView(dom, { + state, + dispatchTransaction(tr) { view.updateState(view.state.apply(tr)); }, + }); + return view; +} + // avoids fragile hand-computed offsets once more than one node's size is involved export function posOf(doc, match) { let result; From 2ced92c0642924c895cd1bcfe810337ffdfa1016 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Fri, 24 Jul 2026 10:33:56 -0700 Subject: [PATCH 08/21] chore(canvas): tighten verbose why-comments Several comments from the previous commit restated the same point across multiple lines instead of saying it once, directly. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/blocks.js | 17 +++++------------ .../canvas/ew-page-outline/ew-page-outline.js | 6 ++---- .../blocks/canvas/editor-utils/blocks.test.js | 5 +---- .../ew-page-outline/ew-page-outline.test.js | 4 +--- test/unit/blocks/canvas/test-helpers.js | 3 +-- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/blocks/canvas/editor-utils/blocks.js b/blocks/canvas/editor-utils/blocks.js index 9dc291d9f..06fe11395 100644 --- a/blocks/canvas/editor-utils/blocks.js +++ b/blocks/canvas/editor-utils/blocks.js @@ -92,11 +92,8 @@ export function deleteBlock(view, blockIndex) { view.dispatch(view.state.tr.delete(pos, pos + node.nodeSize)); } -// proseIndex is a position INSIDE the default-content node (getInstrumentedHTML stamps -// it at the node's content-start, e.g. where an image or a paragraph's text begins), not -// the node's own start — resolving to the depth-1 ancestor recovers the whole top-level -// node regardless of kind or how deeply proseIndex sits inside it (e.g. a blockquote's -// nested paragraph, or a list's first item). +// proseIndex sits inside the node's content, not at its own start; depth-1 recovers +// the whole node regardless of kind or nesting (e.g. a blockquote's nested paragraph). export function getContentItemRange(doc, child) { const pos = doc.resolve(child.proseIndex).before(1); const node = doc.nodeAt(pos); @@ -245,9 +242,7 @@ function getBlockRange(view, blockIndex) { return node ? { pos, size: node.nodeSize, node } : null; } -// Block dropped onto/near a default-content row — the reverse of moveContentItem's -// 'content' target. Block-to-block reordering stays on moveBlock; this only covers a -// block landing next to a content item. +// Reverse of moveContentItem's 'content' target — a block landing next to a content item. export function moveBlockToContentItem(view, blockIndex, targetChild, dropPosition) { if (!view) return; const from = getBlockRange(view, blockIndex); @@ -260,10 +255,8 @@ export function moveBlockToContentItem(view, blockIndex, targetChild, dropPositi spliceNode(view, from, insertPos); } -// Block dropped onto a section's header when that section has no blocks to anchor a -// drop indicator on — the reverse of moveContentItem's 'section' target. Whole-section -// reordering stays on moveSection; this only covers a lone block landing at the -// section boundary. +// Reverse of moveContentItem's 'section' target — a lone block landing at the boundary +// of a section with no blocks to anchor on. export function moveBlockToSection(view, blockIndex, sectionIndex, dropPosition) { if (!view) return; const from = getBlockRange(view, blockIndex); diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index d0bbc515a..6e4c1377d 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -194,8 +194,7 @@ class EwPageOutline extends LitElement { this._setDropIndicator(el, { sectionIndex: sec.sectionIndex, dropPosition }); } else if (type === OUTLINE_TYPES.CONTENT) { - // Bubbles here from anywhere unclaimed in the section; on the header itself we're - // before/after-aware, elsewhere (e.g. an empty section) we default to "first item". + // Bubbles here from anywhere unclaimed in the section; only the header is before/after-aware. e.preventDefault(); const headerEl = e.currentTarget.querySelector('[data-section-header]'); const onHeader = headerEl?.contains(e.target); @@ -216,8 +215,7 @@ class EwPageOutline extends LitElement { return; } - // No blocks in this section to anchor on (it may still have content, or be - // wholly empty) — fall back to the section boundary itself. + // No blocks to anchor on — fall back to the section boundary itself. e.preventDefault(); const headerEl = e.currentTarget.querySelector('[data-section-header]'); this._setDropIndicator( diff --git a/test/unit/blocks/canvas/editor-utils/blocks.test.js b/test/unit/blocks/canvas/editor-utils/blocks.test.js index d7c09b0b9..93f1ebb4b 100644 --- a/test/unit/blocks/canvas/editor-utils/blocks.test.js +++ b/test/unit/blocks/canvas/editor-utils/blocks.test.js @@ -19,10 +19,7 @@ before(async () => { ({ getInstrumentedHTML, parseSections } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); }); -// Builds the same `child` descriptors the outline actually drags/deletes — proseIndex -// comes from the real getInstrumentedHTML/parseSections pipeline, not a hand-picked -// node position, since that's what previously masked a whole class of bugs (proseIndex -// points inside a node's content, not at its own start — see getContentItemRange). +// Real pipeline, not a hand-picked node position — that mismatch is what masked the bug. function childrenOf(view) { const html = getInstrumentedHTML(view); const sections = parseSections(html); diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index 503836d0c..dc16a703b 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -16,9 +16,7 @@ before(async () => { ({ getExtensionsBridge } = await import('../../../../../blocks/canvas/editor-utils/extensions-bridge.js')); }); -// Builds the same `child` descriptors the outline actually drags/deletes — proseIndex -// comes from the real getInstrumentedHTML/parseSections pipeline, not a hand-picked -// node position (proseIndex points inside a node's content, not at its own start). +// Real pipeline, not a hand-picked node position — matches what the outline actually does. function childrenOf(view) { const html = getInstrumentedHTML(view); const sections = parseSections(html); diff --git a/test/unit/blocks/canvas/test-helpers.js b/test/unit/blocks/canvas/test-helpers.js index eeb098dd6..9381a6f0a 100644 --- a/test/unit/blocks/canvas/test-helpers.js +++ b/test/unit/blocks/canvas/test-helpers.js @@ -12,8 +12,7 @@ export function makeView(json) { }; } -// A real, mounted EditorView — needed wherever a test goes through getInstrumentedHTML, -// since that relies on view.posAtDOM/view.dom, which a fake view can't provide. +// Needed wherever a test goes through getInstrumentedHTML, which relies on view.posAtDOM/view.dom. export function makeRealView(json) { const doc = schema.nodeFromJSON(json); const state = EditorState.create({ schema, doc }); From 547a7276ca9931daf8950cf6e168667e9863d484 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Fri, 24 Jul 2026 10:46:48 -0700 Subject: [PATCH 09/21] fix(canvas): highlight the selected default-content item in the outline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content children had no selected-state feedback, unlike blocks — clicking one scrolled the doc but left the outline row unstyled. Tracks the last clicked content proseIndex, clearing it whenever a block is selected instead, and reuses the existing .block-item.selected styling. Co-Authored-By: Claude Sonnet 5 --- .../canvas/ew-page-outline/ew-page-outline.js | 11 ++++++++++- .../ew-page-outline/ew-page-outline.test.js | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 6e4c1377d..e7a0feef3 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -75,6 +75,7 @@ class EwPageOutline extends LitElement { static properties = { _sections: { state: true }, _selectedBlockIndex: { state: true }, + _selectedProseIndex: { state: true }, _hashState: { state: true }, _hasBlockLibrary: { state: true }, _expandedContent: { state: true }, @@ -92,12 +93,14 @@ class EwPageOutline extends LitElement { } else { this._sections = undefined; this._selectedBlockIndex = undefined; + this._selectedProseIndex = undefined; } }); this._unsubscribeSelect = editorSelectChange .subscribe(({ blockIndex, source }) => { if (source === 'outline') return; this._selectedBlockIndex = blockIndex; + this._selectedProseIndex = undefined; }); } @@ -118,6 +121,7 @@ class EwPageOutline extends LitElement { if (this._prevSelectedPath !== undefined && sp !== this._prevSelectedPath) { this._sections = undefined; this._selectedBlockIndex = undefined; + this._selectedProseIndex = undefined; } this._prevSelectedPath = sp; @@ -138,10 +142,13 @@ class EwPageOutline extends LitElement { _select(blockIndex) { this._selectedBlockIndex = blockIndex; + this._selectedProseIndex = undefined; editorSelectChange.emit({ blockIndex, source: 'outline' }); } _selectProse(proseIndex, kind) { + this._selectedProseIndex = proseIndex; + this._selectedBlockIndex = undefined; editorProseSelectChange.emit({ proseIndex, kind }); } @@ -374,7 +381,9 @@ class EwPageOutline extends LitElement { ${expanded ? html`

      ${item.children.map((child) => html` -
    • this._onDragStart(e, OUTLINE_TYPES.CONTENT, child)} @dragover=${(e) => this._onContentDragOver(e, child)} diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index dc16a703b..196289467 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -123,6 +123,25 @@ describe('ew-page-outline — expandable default content', () => { expect(received).to.deep.equal({ proseIndex: 9, kind: 'image' }); }); + it('marks a clicked content child as selected, and clears it when a block is selected instead', async () => { + el._sections[0].blocks = [{ name: 'hero', blockIndex: 0 }]; + el.shadowRoot.querySelector('.content-item').click(); + await el.updateComplete; + + const paragraphChild = [...el.shadowRoot.querySelectorAll('.content-child')][1]; + paragraphChild.click(); + await el.updateComplete; + + expect(paragraphChild.classList.contains('selected')).to.be.true; + expect(paragraphChild.getAttribute('aria-selected')).to.equal('true'); + + el._select(0); + await el.updateComplete; + + expect(paragraphChild.classList.contains('selected')).to.be.false; + expect(paragraphChild.getAttribute('aria-selected')).to.equal('false'); + }); + it('expands and collapses the focused group header with ArrowRight/ArrowLeft', async () => { const header = el.shadowRoot.querySelector('.content-item'); header.focus(); From 8fb989115ca48255db6452ab55cc3dadcf21ae18 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Tue, 28 Jul 2026 16:24:48 -0700 Subject: [PATCH 10/21] fix(canvas): restore grab cursor on default-content drag handles .content-item's pointer cursor (for the group-toggle header) was overriding .block-item's grab cursor on draggable content-child rows, since it comes later in the cascade. Scope it to the header only. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/ew-page-outline/ew-page-outline.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.css b/blocks/canvas/ew-page-outline/ew-page-outline.css index 2aac50f08..b4fd79ddb 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.css +++ b/blocks/canvas/ew-page-outline/ew-page-outline.css @@ -121,7 +121,7 @@ padding: 0; } -.content-item { +.content-item:not(.content-child) { cursor: pointer; } From b59aa590300ce43645ab9c2b99a027d75bc76c66 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 08:09:31 -0700 Subject: [PATCH 11/21] fix(canvas): align outline rows and show content-child text previews Blocks and content-group headers now start at the same left edge (blocks previously lacked the chevron's reserved indent). Content-child rows also show a truncated text snippet under the type label so similarly-typed nodes (Paragraph, Heading, etc.) are distinguishable. Co-Authored-By: Claude Sonnet 5 --- .../ew-page-outline/ew-page-outline.css | 23 +++++++++++++++++++ .../canvas/ew-page-outline/ew-page-outline.js | 5 +++- .../ew-page-outline/ew-page-outline.test.js | 4 ++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.css b/blocks/canvas/ew-page-outline/ew-page-outline.css index b4fd79ddb..d43dd1db9 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.css +++ b/blocks/canvas/ew-page-outline/ew-page-outline.css @@ -106,6 +106,11 @@ border-bottom: 2px solid var(--s2-blue-600, #147af3); } +/* Blocks have no chevron — indent to align with expandable content-item labels */ +.block-item:not(.content-item) { + padding-inline-start: calc(var(--s2-spacing-300) + 1rem); +} + .block-empty { cursor: default; } @@ -146,6 +151,7 @@ .content-child { padding-inline-start: var(--s2-spacing-500); + min-height: 44px; } .content-label { @@ -154,6 +160,23 @@ color: var(--s2-gray-700); } +.content-label-stack { + flex: 1; + display: flex; + flex-direction: column; + min-width: 0; + overflow: hidden; +} + +.content-snippet { + font-size: var(--s2-body-size-xs, 0.75rem); + font-weight: 400; + color: var(--s2-gray-600); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + .block-name { flex: 1; overflow: hidden; diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index e7a0feef3..5c3bbbe3b 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -390,7 +390,10 @@ class EwPageOutline extends LitElement { @drop=${this._onDrop} @dragend=${this._onDragEnd} @click=${(e) => { e.stopPropagation(); this._selectProse(child.proseIndex, child.kind); }}> - + ${this._renderDeleteButton(OUTLINE_TYPES.CONTENT, child)}
      ) and relies on existing CSS text-overflow: ellipsis for visual truncation. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/editor-utils.js | 13 +++++++++++++ blocks/canvas/ew-page-outline/ew-page-outline.js | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 2bd2372ed..caa07e17f 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -284,6 +284,18 @@ function getDefaultContentProseIndex(el, kind) { return attr != null ? Number(attr) : undefined; } +function firstLineText(el) { + const clone = el.cloneNode(true); + clone.querySelectorAll('br').forEach((br) => br.replaceWith('\n')); + return clone.textContent.trim().split('\n')[0].trim(); +} + +function getContentSnippet(el, kind) { + if (kind === 'list') return firstLineText(el.querySelector(':scope > li') ?? el); + if (kind === 'quote') return firstLineText(el.querySelector(':scope > p') ?? el); + return firstLineText(el); +} + function getDefaultContentKind(el) { const tag = el.tagName; if (/^H[1-6]$/.test(tag)) return { kind: 'heading', level: Number(tag[1]) }; @@ -318,6 +330,7 @@ export function parseSections(htmlText) { ...kindInfo, proseIndex: getDefaultContentProseIndex(el, kindInfo.kind), innerText: el.textContent.trim(), + snippet: getContentSnippet(el, kindInfo.kind), }; }), }); diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index b3782de2a..8cb1115cf 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -392,7 +392,7 @@ class EwPageOutline extends LitElement { @click=${(e) => { e.stopPropagation(); this._selectProse(child.proseIndex, child.kind); }}> ${this._renderDeleteButton(OUTLINE_TYPES.CONTENT, child)}

      tags alive through prose2aem's canvas-only instrumented-HTML path (keepEmptyParagraphs), and fixing getDefaultContentKind's "no text = image" heuristic, which broke once genuinely empty (imageless) paragraphs could reach it. - Drag/drop reorder now selects the moved item (via tr.setSelection inside the same transaction); deleting a content item selects the next/previous sibling in the same run; block/section delete leave selection untouched. - Fixed a pre-existing bug where handleCursorMove's redundant blockIndex-only editorSelectChange emit clobbered the correct proseIndex-carrying 'doc' emit immediately after it, breaking outline sync on the first WYSIWYG cursor move. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/blocks.js | 52 +++++-- blocks/canvas/editor-utils/editor-utils.js | 15 +- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 1 - .../ew-editor-wysiwyg/utils/handlers.js | 11 +- .../canvas/ew-page-outline/ew-page-outline.js | 37 ++++- blocks/shared/prose2aem.js | 16 +- .../blocks/canvas/editor-utils/blocks.test.js | 138 ++++++++++++++++++ .../canvas/editor-utils/editor-utils.test.js | 79 +++++++--- .../ew-page-outline/ew-page-outline.test.js | 34 +++-- 9 files changed, 318 insertions(+), 65 deletions(-) diff --git a/blocks/canvas/editor-utils/blocks.js b/blocks/canvas/editor-utils/blocks.js index 06fe11395..88601f6d6 100644 --- a/blocks/canvas/editor-utils/blocks.js +++ b/blocks/canvas/editor-utils/blocks.js @@ -1,4 +1,4 @@ -import { DOMParser as PMDOMParser } from 'da-y-wrapper'; +import { DOMParser as PMDOMParser, NodeSelection } from 'da-y-wrapper'; const NON_BLOCK_TABLE_NAMES = new Set(['metadata', 'section metadata', 'section-metadata']); @@ -44,15 +44,16 @@ export function getActiveBlockIndex(view) { return -1; } -// Shared by every single-node move; adjusts insertPos for the shift the delete causes. +// Shared by every single-node move; adjusts insertPos for the shift the delete causes, +// and selects the moved node at its new position. function spliceNode(view, from, insertPos) { const adjustedInsertPos = insertPos > from.pos ? insertPos - from.size : insertPos; if (adjustedInsertPos === from.pos) return; - view.dispatch( - view.state.tr - .delete(from.pos, from.pos + from.size) - .insert(adjustedInsertPos, from.node), - ); + const tr = view.state.tr + .delete(from.pos, from.pos + from.size) + .insert(adjustedInsertPos, from.node); + tr.setSelection(NodeSelection.create(tr.doc, adjustedInsertPos)); + view.dispatch(tr); } export function moveBlock(view, fromIndex, toIndex, dropPosition) { @@ -100,11 +101,34 @@ export function getContentItemRange(doc, child) { return node ? { pos, size: node.nodeSize, node } : null; } +function isContentNode(node, schema) { + return node.type.name !== 'table' && node.type !== schema.nodes.horizontal_rule; +} + +// Same-run adjacency: the immediate top-level sibling in a given direction, or null if +// it doesn't exist or is a block/section boundary (which ends the run). +function getRunSibling(doc, schema, pos, direction) { + const topNodes = []; + doc.forEach((node, nodePos) => topNodes.push({ node, pos: nodePos })); + const idx = topNodes.findIndex((entry) => entry.pos === pos); + if (idx === -1) return null; + const sibling = topNodes[idx + direction]; + if (!sibling || !isContentNode(sibling.node, schema)) return null; + return sibling; +} + export function deleteContentItem(view, child) { if (!view) return; - const range = getContentItemRange(view.state.doc, child); + const { doc, schema } = view.state; + const range = getContentItemRange(doc, child); if (!range) return; - view.dispatch(view.state.tr.delete(range.pos, range.pos + range.size)); + + const sibling = getRunSibling(doc, schema, range.pos, 1) + ?? getRunSibling(doc, schema, range.pos, -1); + + const tr = view.state.tr.delete(range.pos, range.pos + range.size); + if (sibling) tr.setSelection(NodeSelection.create(tr.doc, tr.mapping.map(sibling.pos))); + view.dispatch(tr); } function getSectionStartOffset(view, sectionIndex) { @@ -180,12 +204,20 @@ export function moveSection(view, fromSectionIndex, toSectionIndex, dropPosition const hrNode = schema.nodes.horizontal_rule.create(); const newNodes = []; + let movedSectionStart; reordered.forEach((sectionNodes, i) => { if (i > 0) newNodes.push(hrNode); + if (sectionNodes === moved) { + movedSectionStart = newNodes.reduce((size, node) => size + node.nodeSize, 0); + } newNodes.push(...sectionNodes); }); - view.dispatch(view.state.tr.replaceWith(0, doc.content.size, newNodes)); + const tr = view.state.tr.replaceWith(0, doc.content.size, newNodes); + if (movedSectionStart != null && moved.length) { + tr.setSelection(NodeSelection.create(tr.doc, movedSectionStart)); + } + view.dispatch(tr); } // Counterpart to getSectionStartOffset — the hr position bounding the previous section. diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index caa07e17f..b75282ab1 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -253,6 +253,8 @@ export function getInstrumentedHTML(view) { // Serialize clone to HTML, then move block-marker index onto wrapper as data-block-index // (same pattern as da-nx qe-advanced: getInstrumentedHTML in prose2aem.js). + // keepEmptyParagraphs: an empty

      being actively edited must survive here so + // parseSections/the outline can see and select it; the saved/published HTML still strips it. let htmlString = prose2aem(editorClone, true, false, true); htmlString = htmlString.replace( /

      <\/div>\s*]*?)>/gi, @@ -303,9 +305,11 @@ function getDefaultContentKind(el) { if (tag === 'UL') return { kind: 'list', ordered: false }; if (tag === 'PRE') return { kind: 'code' }; if (tag === 'BLOCKQUOTE') return { kind: 'quote' }; - // a text-less

      wraps only an image, as does a bare /; - // anything with text is a paragraph - return { kind: el.textContent?.trim() ? 'paragraph' : 'image' }; + if (el.textContent?.trim()) return { kind: 'paragraph' }; + // A text-less

      wraps only an image, as does a bare / — but a text-less + //

      with no image at all is just an empty paragraph, not an image wrapper. + const hasImage = el.matches?.('img') || !!el.querySelector?.('img'); + return { kind: hasImage ? 'image' : 'paragraph' }; } export function parseSections(htmlText) { @@ -331,6 +335,7 @@ export function parseSections(htmlText) { proseIndex: getDefaultContentProseIndex(el, kindInfo.kind), innerText: el.textContent.trim(), snippet: getContentSnippet(el, kindInfo.kind), + empty: !hasDefaultContent(el), }; }), }); @@ -354,9 +359,7 @@ export function parseSections(htmlText) { return; } - // Skip empty nodes — prose2aem doesn't always strip them (e.g. an empty

      can - // survive serialization) — so they neither break nor join a run. - if (hasDefaultContent(el)) currentRun.push(el); + currentRun.push(el); }); flushRun(); diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index 1ea38ab03..ad08bc7eb 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -223,7 +223,6 @@ export class EwEditorDoc extends LitElement { port: this.quickEditPort, iframe: this._wysiwygIframe, suppressRerender: false, - lastBlockIndex: undefined, owner: org, repo, path: controllerPathnameFromEditorCtx(this.ctx), diff --git a/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js b/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js index 37b6a59fe..2f1e1e9bf 100644 --- a/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js +++ b/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js @@ -5,8 +5,6 @@ import { 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 { getActiveBlockIndex } from '../../editor-utils/blocks.js'; export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { const { view, wsProvider } = ctx; @@ -59,6 +57,10 @@ export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { } ctx.suppressRerender = true; + // dispatch() synchronously runs createTrackingPlugin's view-update hook, which already + // emits editorSelectChange with the full { blockIndex, proseIndex, source: 'doc' } + // payload for this exact selection — a second, blockIndex-only emit here would + // clobber that (dropping proseIndex) and incorrectly collapse the outline. view.dispatch(tr.scrollIntoView()); ctx.suppressRerender = false; const tb = getSelectionToolbar(); @@ -66,11 +68,6 @@ export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { tb.view = view; tb.show(); } - const blockIndex = getActiveBlockIndex(view); - if (blockIndex !== ctx.lastBlockIndex) { - ctx.lastBlockIndex = blockIndex; - editorSelectChange.emit({ blockIndex, source: 'wysiwyg' }); - } } catch (error) { // eslint-disable-next-line no-console console.error('Error moving cursor:', error); diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 8cb1115cf..813eb6da2 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -37,7 +37,8 @@ const DROP_POSITIONS = { function contentChildEqual(child, other) { return child.proseIndex === other.proseIndex && child.innerText === other.innerText - && child.kind === other.kind && child.level === other.level && child.ordered === other.ordered; + && child.kind === other.kind && child.level === other.level && child.ordered === other.ordered + && child.empty === other.empty; } function contentChildLabel(child) { @@ -101,6 +102,9 @@ class EwPageOutline extends LitElement { if (source === 'outline') return; this._selectedBlockIndex = blockIndex; this._selectedProseIndex = proseIndex; + if (blockIndex != null && blockIndex >= 0) this._resetExpansionForBlock(); + else if (proseIndex != null) this._resetExpansionForProse(proseIndex); + else this._resetExpansionForBlock(); }); } @@ -143,12 +147,14 @@ class EwPageOutline extends LitElement { _select(blockIndex) { this._selectedBlockIndex = blockIndex; this._selectedProseIndex = undefined; + this._resetExpansionForBlock(); editorSelectChange.emit({ blockIndex, source: 'outline' }); } _selectProse(proseIndex, kind) { this._selectedProseIndex = proseIndex; this._selectedBlockIndex = undefined; + this._resetExpansionForProse(proseIndex); editorProseSelectChange.emit({ proseIndex, kind }); } @@ -159,6 +165,29 @@ class EwPageOutline extends LitElement { this._expandedContent = next; } + // Selection changes always fully reset expansion (see _renderContentGroup): a block + // selection collapses every run, a content selection expands only its own run. Manual + // expand/collapse (_toggleContentGroup) persists only until the next selection change. + _resetExpansionForBlock() { + this._expandedContent = new Set(); + } + + _resetExpansionForProse(proseIndex) { + const runKey = this._findRunKeyForProseIndex(proseIndex); + this._expandedContent = runKey != null ? new Set([runKey]) : new Set(); + } + + _findRunKeyForProseIndex(proseIndex) { + for (const sec of this._sections ?? []) { + for (const item of sec.items) { + if (item.type === 'content' && item.children.some((child) => child.proseIndex === proseIndex)) { + return item.proseIndex; + } + } + } + return undefined; + } + _clearDropIndicator() { this.shadowRoot.querySelector('[data-drop-position]')?.removeAttribute('data-drop-position'); } @@ -366,6 +395,10 @@ class EwPageOutline extends LitElement { } _renderContentGroup(item, isFirst) { + const visibleChildren = item.children.filter( + (child) => !child.empty || child.proseIndex === this._selectedProseIndex, + ); + if (!visibleChildren.length) return nothing; const key = item.proseIndex; const expanded = this._expandedContent?.has(key); return html` @@ -380,7 +413,7 @@ class EwPageOutline extends LitElement {

      ${expanded ? html`
        - ${item.children.map((child) => html` + ${visibleChildren.map((child) => html`
      • p'); paras.forEach((p) => { // Remove empty p tags - if (p.innerHTML.trim() === '') { p.remove(); } + if (!keepEmpty && p.innerHTML.trim() === '') { p.remove(); } // Convert dash p tags to rules if (p.textContent.trim() === '---') { const hr = document.createElement('hr'); @@ -263,9 +263,17 @@ function convertLocalUrlsToRelative(editor) { * @param {HTMLElement} editor the editor dom * @param {Boolean} livePreview whether or not the target destination is Live Preview * @param {Boolean} isFragment whether or not the DOM is a fragment + * @param {Boolean} keepEmptyParagraphs keep empty

        tags instead of stripping them + * (canvas's instrumented HTML needs these so an empty node being edited stays + * visible/selectable in the outline; the actual saved/published HTML still strips them) * @returns AEM-friendly HTML as a text string */ -export default function prose2aem(editor, livePreview, isFragment = false) { +export default function prose2aem( + editor, + livePreview, + isFragment = false, + keepEmptyParagraphs = false, +) { if (!isFragment) editor.removeAttribute('class'); editor.removeAttribute('contenteditable'); @@ -297,7 +305,7 @@ export default function prose2aem(editor, livePreview, isFragment = false) { convertListItems(editor); - convertParagraphs(editor); + convertParagraphs(editor, keepEmptyParagraphs); convertBlocks(editor, isFragment); diff --git a/test/unit/blocks/canvas/editor-utils/blocks.test.js b/test/unit/blocks/canvas/editor-utils/blocks.test.js index 93f1ebb4b..411008514 100644 --- a/test/unit/blocks/canvas/editor-utils/blocks.test.js +++ b/test/unit/blocks/canvas/editor-utils/blocks.test.js @@ -1,12 +1,15 @@ +import { NodeSelection } from 'da-y-wrapper'; import { expect } from '@esm-bundle/chai'; import { setNx } from '../../../../../scripts/utils.js'; import { getContentItemRange, deleteContentItem, + deleteBlock, moveContentItem, moveBlockToContentItem, moveBlockToSection, moveBlock, + moveSection, } from '../../../../../blocks/canvas/editor-utils/blocks.js'; import { makeView, makeRealView } from '../test-helpers.js'; @@ -144,6 +147,59 @@ describe('deleteContentItem', () => { expect(view.state.doc.childCount).to.equal(1); expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); }); + + it('selects the next sibling in the same run after deleting a middle child', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'C' }] }, + ], + }); + const child = childrenOf(view).find((c) => c.innerText === 'B'); + deleteContentItem(view, child); + + const { selection, doc } = view.state; + expect(selection).to.be.instanceOf(NodeSelection); + expect(selection.node.textContent).to.equal('C'); + expect(doc.childCount).to.equal(2); + }); + + it('falls back to the previous sibling when the deleted child was last in its run', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'C' }] }, + ], + }); + const child = childrenOf(view).find((c) => c.innerText === 'C'); + deleteContentItem(view, child); + + const { selection } = view.state; + expect(selection).to.be.instanceOf(NodeSelection); + expect(selection.node.textContent).to.equal('B'); + }); + + it('never crosses a block boundary to find a sibling to select', () => { + const view = makeRealView({ + type: 'doc', + content: [ + tableJSON('hero'), + { type: 'paragraph', content: [{ type: 'text', text: 'Lone para' }] }, + tableJSON('cards'), + ], + }); + const child = childrenOf(view).find((c) => c.innerText === 'Lone para'); + deleteContentItem(view, child); + + // No same-run sibling exists (blocks on both sides) — nothing gets deliberately + // selected, so the neighboring tables are never turned into a NodeSelection. + expect(view.state.selection).to.not.be.instanceOf(NodeSelection); + expect(view.state.doc.childCount).to.equal(2); + }); }); describe('moveContentItem', () => { @@ -238,6 +294,25 @@ describe('moveContentItem', () => { expect(view.state).to.equal(before); }); + + it('selects the moved item at its new position', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + const [a, b] = childrenOf(view); + + moveContentItem(view, a, { type: 'content', child: b }, 'after'); + + const { selection } = view.state; + expect(selection).to.be.instanceOf(NodeSelection); + expect(selection.node.textContent).to.equal('A'); + expect(docTypes(view.state.doc)).to.deep.equal(['paragraph', 'paragraph']); + expect(view.state.doc.lastChild.textContent).to.equal('A'); + }); }); describe('moveBlockToContentItem', () => { @@ -333,4 +408,67 @@ describe('moveBlock (regression coverage for the shared splice helper)', () => { }); expect(names).to.deep.equal(['columns', 'hero', 'cards']); }); + + it('selects the moved block at its new position', () => { + const view = makeRealView({ + type: 'doc', + content: [tableJSON('hero'), tableJSON('cards')], + }); + moveBlock(view, 0, 1, 'after'); + + const { selection } = view.state; + expect(selection).to.be.instanceOf(NodeSelection); + expect(selection.node.type.name).to.equal('table'); + expect(selection.node.firstChild.firstChild.textContent).to.equal('hero'); + }); +}); + +describe('moveSection', () => { + it('selects the first node of the moved section at its new position', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'horizontal_rule' }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + + moveSection(view, 0, 1, 'after'); + + const { doc, selection } = view.state; + expect(docTypes(doc)).to.deep.equal(['paragraph', 'horizontal_rule', 'paragraph']); + // Section 0 (containing 'A') is the one being moved, to land after section 1 ('B'). + expect(doc.firstChild.textContent).to.equal('B'); + expect(selection).to.be.instanceOf(NodeSelection); + expect(selection.node.textContent).to.equal('A'); + }); + + it('does not throw when the moved section is empty', () => { + const view = makeRealView({ + type: 'doc', + content: [ + { type: 'horizontal_rule' }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + + expect(() => moveSection(view, 0, 1, 'after')).to.not.throw(); + // Pre-existing behavior: moving an empty section still leaves its separating hr. + expect(docTypes(view.state.doc)).to.deep.equal(['paragraph', 'horizontal_rule']); + }); +}); + +describe('deleteBlock (no deliberate selection change)', () => { + it('leaves selection to PM\'s own mapping rather than selecting a sibling', () => { + const view = makeRealView({ + type: 'doc', + content: [tableJSON('hero'), tableJSON('cards')], + }); + + deleteBlock(view, 0); + + expect(view.state.selection).to.not.be.instanceOf(NodeSelection); + expect(docTypes(view.state.doc)).to.deep.equal(['table']); + }); }); diff --git a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js index 5e8f496fb..5e5e533fb 100644 --- a/test/unit/blocks/canvas/editor-utils/editor-utils.test.js +++ b/test/unit/blocks/canvas/editor-utils/editor-utils.test.js @@ -155,13 +155,17 @@ describe('parseSections', () => { type: 'content', proseIndex: 1, innerText: 'Intro text', - children: [{ type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Intro text' }], + children: [{ + type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Intro text', snippet: 'Intro text', empty: false, + }], }); expect(section.items[2]).to.deep.equal({ type: 'content', proseIndex: 20, innerText: 'Outro text', - children: [{ type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Outro text' }], + children: [{ + type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Outro text', snippet: 'Outro text', empty: false, + }], }); }); @@ -177,9 +181,15 @@ describe('parseSections', () => { proseIndex: 1, innerText: 'Title Para one Para two', children: [ - { type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title' }, - { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one' }, - { type: 'content', kind: 'paragraph', proseIndex: 12, innerText: 'Para two' }, + { + type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title', snippet: 'Title', empty: false, + }, + { + type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one', snippet: 'Para one', empty: false, + }, + { + type: 'content', kind: 'paragraph', proseIndex: 12, innerText: 'Para two', snippet: 'Para two', empty: false, + }, ], }]); }); @@ -238,13 +248,17 @@ describe('parseSections', () => { const [section] = parseSections(html); const [{ children }] = section.items; expect(children.map((c) => c.kind)).to.deep.equal(['heading', 'code']); - expect(children[1]).to.deep.equal({ type: 'content', kind: 'code', proseIndex: 5, innerText: 'const x = 1;' }); + expect(children[1]).to.deep.equal({ + type: 'content', kind: 'code', proseIndex: 5, innerText: 'const x = 1;', snippet: 'const x = 1;', empty: false, + }); }); - it('treats empty loose nodes as invisible — they neither break nor join a run', () => { + it('keeps empty loose nodes in the run, flagged empty, without breaking it', () => { + // getInstrumentedHTML stamps data-prose-index on every editable element, empty or + // not (keepEmptyParagraphs keeps the empty

        itself alive through prose2aem too). const html = `

        Para one

        -

        +

        Para two

        `; @@ -254,21 +268,33 @@ describe('parseSections', () => { proseIndex: 1, innerText: 'Para one Para two', children: [ - { type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Para one' }, - { type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Para two' }, + { + type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Para one', snippet: 'Para one', empty: false, + }, + { + type: 'content', kind: 'heading', level: 2, proseIndex: 8, innerText: '', snippet: '', empty: true, + }, + { + type: 'content', kind: 'paragraph', proseIndex: 12, innerText: '', snippet: '', empty: true, + }, + { + type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Para two', snippet: 'Para two', empty: false, + }, ], }]); }); - it('produces nothing for a run made up entirely of empty nodes', () => { + it('produces a content entry flagged empty for a run made up entirely of empty nodes', () => { const html = `
        Hero
        -

        -

        +

        +

        `; const [section] = parseSections(html); - expect(section.items).to.have.length(1); + expect(section.items).to.have.length(2); expect(section.items[0].type).to.equal('block'); + expect(section.items[1].type).to.equal('content'); + expect(section.items[1].children.every((c) => c.empty)).to.be.true; }); it('reads proseIndex from data-image-index on a loose image', () => { @@ -280,24 +306,33 @@ describe('parseSections', () => { type: 'content', proseIndex: 7, innerText: '', - children: [{ type: 'content', kind: 'image', proseIndex: 7, innerText: '' }], + children: [{ + type: 'content', kind: 'image', proseIndex: 7, innerText: '', snippet: '', empty: false, + }], }]); }); - it('takes proseIndex from the first non-empty node in a run', () => { + it('takes run identity from the first node in the run, even when that node is empty', () => { const html = `
        -

        +

        First real content

        More content

        `; const [section] = parseSections(html); expect(section.items).to.deep.equal([{ type: 'content', - proseIndex: 9, + proseIndex: 1, innerText: 'First real content More content', children: [ - { type: 'content', kind: 'paragraph', proseIndex: 9, innerText: 'First real content' }, - { type: 'content', kind: 'paragraph', proseIndex: 15, innerText: 'More content' }, + { + type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: '', snippet: '', empty: true, + }, + { + type: 'content', kind: 'paragraph', proseIndex: 9, innerText: 'First real content', snippet: 'First real content', empty: false, + }, + { + type: 'content', kind: 'paragraph', proseIndex: 15, innerText: 'More content', snippet: 'More content', empty: false, + }, ], }]); }); @@ -313,7 +348,9 @@ describe('parseSections', () => { type: 'content', proseIndex: 1, innerText: 'Section one text', - children: [{ type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Section one text' }], + children: [{ + type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Section one text', snippet: 'Section one text', empty: false, + }], }]); expect(sections[1].items).to.deep.equal([ { type: 'block', name: 'cards', blockIndex: 0, proseIndex: 0, innerText: 'Cards' }, diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index 6e1ac8f05..5549b012c 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -59,11 +59,15 @@ describe('ew-page-outline — expandable default content', () => { blocks: [], items: [ contentGroupItem(1, [ - { type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title' }, - { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one' }, - { type: 'content', kind: 'image', proseIndex: 9, innerText: '' }, - { type: 'content', kind: 'list', ordered: true, proseIndex: 12, innerText: 'one two' }, - { type: 'content', kind: 'code', proseIndex: 15, innerText: 'const x = 1;' }, + { + type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title', snippet: 'Title', + }, + { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one', snippet: 'Para one' }, + { type: 'content', kind: 'image', proseIndex: 9, innerText: '', snippet: '' }, + { + type: 'content', kind: 'list', ordered: true, proseIndex: 12, innerText: 'one two', snippet: 'one two', + }, + { type: 'content', kind: 'code', proseIndex: 15, innerText: 'const x = 1;', snippet: 'const x = 1;' }, ]), ], }]; @@ -124,7 +128,7 @@ describe('ew-page-outline — expandable default content', () => { expect(received).to.deep.equal({ proseIndex: 9, kind: 'image' }); }); - it('marks a clicked content child as selected, and clears it when a block is selected instead', async () => { + it('marks a clicked content child as selected, and collapses its run when a block is selected instead', async () => { el._sections[0].blocks = [{ name: 'hero', blockIndex: 0 }]; el.shadowRoot.querySelector('.content-item').click(); await el.updateComplete; @@ -139,24 +143,26 @@ describe('ew-page-outline — expandable default content', () => { el._select(0); await el.updateComplete; - expect(paragraphChild.classList.contains('selected')).to.be.false; - expect(paragraphChild.getAttribute('aria-selected')).to.equal('false'); + // Selecting a block resets expansion, collapsing every run — the previously + // selected child is no longer rendered at all, not just unmarked as selected. + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('false'); + expect(el.shadowRoot.querySelectorAll('.content-child')).to.have.lengthOf(0); }); - it('highlights a content child when the doc selection (not just an outline click) lands on it', async () => { - el.shadowRoot.querySelector('.content-item').click(); - await el.updateComplete; - const paragraphChild = [...el.shadowRoot.querySelectorAll('.content-child')][1]; - + it('highlights a content child when the doc selection (not just an outline click) lands on it, and collapses its run on block selection', async () => { editorSelectChange.emit({ blockIndex: -1, proseIndex: 5, source: 'doc' }); await el.updateComplete; + // A content selection expands only the run containing it, with no manual click needed. + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + const paragraphChild = [...el.shadowRoot.querySelectorAll('.content-child')][1]; expect(paragraphChild.classList.contains('selected')).to.be.true; editorSelectChange.emit({ blockIndex: 0, proseIndex: undefined, source: 'doc' }); await el.updateComplete; - expect(paragraphChild.classList.contains('selected')).to.be.false; + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('false'); + expect(el.shadowRoot.querySelectorAll('.content-child')).to.have.lengthOf(0); }); it('expands and collapses the focused group header with ArrowRight/ArrowLeft', async () => { From bcc8119978016bc4d78ce2711dc92452eab826f6 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 11:19:54 -0700 Subject: [PATCH 16/21] docs(canvas): trim PR #1167 comments to minimum load-bearing wording Cut redundant/verbose comments added in this PR: dropped one that restated what the name/body already conveyed, deduped a proseIndex convention explained twice across files, and tightened several multi-line comments down to their essential point. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/blocks.js | 2 -- blocks/canvas/editor-utils/editor-utils.js | 3 +-- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 21 +++++++------------ .../canvas/ew-editor-doc/utils/selection.js | 11 ++++------ .../ew-editor-wysiwyg/utils/handlers.js | 7 +++---- .../canvas/ew-page-outline/ew-page-outline.js | 5 ++--- .../ew-editor-doc/ew-editor-doc.test.js | 8 +++---- 7 files changed, 20 insertions(+), 37 deletions(-) diff --git a/blocks/canvas/editor-utils/blocks.js b/blocks/canvas/editor-utils/blocks.js index 88601f6d6..67fd2f634 100644 --- a/blocks/canvas/editor-utils/blocks.js +++ b/blocks/canvas/editor-utils/blocks.js @@ -105,8 +105,6 @@ function isContentNode(node, schema) { return node.type.name !== 'table' && node.type !== schema.nodes.horizontal_rule; } -// Same-run adjacency: the immediate top-level sibling in a given direction, or null if -// it doesn't exist or is a block/section boundary (which ends the run). function getRunSibling(doc, schema, pos, direction) { const topNodes = []; doc.forEach((node, nodePos) => topNodes.push({ node, pos: nodePos })); diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index b75282ab1..2e4ab584d 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -421,8 +421,7 @@ export const editorSelectChange = (() => { })(); // Event observable — no replay on subscribe. See docs/canvas-events.md. -// Selects/scrolls to a raw ProseMirror position (not a block index); used by the -// outline's default-content entries. +// Carries a raw ProseMirror position, not a block index, for the outline's default-content entries. export const editorProseSelectChange = (() => { const listeners = new Set(); return { diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index ad08bc7eb..8cefc0249 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -130,13 +130,9 @@ export class EwEditorDoc extends LitElement { view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); } - // proseIndex may point mid-node (or have drifted since the outline was built), so - // TextSelection.near is the fallback that resolves to the nearest valid selection - // without throwing. When the node at proseIndex still matches the outline kind, select - // it as a NodeSelection instead so it gets the same blue-border highlight as a block. - // Either way, broadcast so the layout view (quick-edit) can scroll/highlight the match - // (using the raw, unadjusted proseIndex — that's what data-prose-index in the layout - // DOM actually carries; see the nodeStart comment below for why it differs here). + // TextSelection.near is the fallback for a drifted/mid-node proseIndex. A kind match + // selects a NodeSelection instead, for the block-style highlight. Either way, broadcasts + // the raw proseIndex, since that's what layout-view's data-prose-index carries. _scrollDocToProseIndex(proseIndex, kind) { if (proseIndex == null || proseIndex < 0) return; const { view } = this._proseContext ?? {}; @@ -151,10 +147,8 @@ export class EwEditorDoc extends LitElement { return; } - // Unlike images, non-image content's proseIndex (from data-prose-index/posAtDOM) is one - // position *inside* the node's own start — da-nx's inline-editor bootstrap depends on - // that exact convention (see cursorOffset in prose-diff.js/da-nx's prose.js), so it can't - // change. Step back one to get the node's own start for a NodeSelection anchor. + // Non-image content's proseIndex is one position inside the node's own start + // (see activeContentProseIndex in utils/selection.js) — step back one for the anchor. const nodeStart = proseIndex - 1; const nodeNames = CONTENT_KIND_NODE_NAMES[kind]; if (nodeStart >= 0 && nodeNames?.includes(doc.nodeAt(nodeStart)?.type.name)) { @@ -169,9 +163,8 @@ export class EwEditorDoc extends LitElement { this._broadcastSelectedNode(true, { anchorType: 'content', proseIndex }); } - // overrideNode lets outline-driven content navigation (a TextSelection, which - // selectedNodePayload can't classify) broadcast an explicit anchorType/proseIndex - // instead of one derived from the current ProseMirror selection. + // overrideNode lets content navigation (a TextSelection selectedNodePayload can't + // classify) broadcast an explicit anchorType/proseIndex instead of a derived one. _broadcastSelectedNode(scrollIntoView = false, overrideNode = undefined) { const port = this._controllerCtx?.port; const { view } = this._proseContext ?? {}; diff --git a/blocks/canvas/ew-editor-doc/utils/selection.js b/blocks/canvas/ew-editor-doc/utils/selection.js index 2fef04420..6dca0d935 100644 --- a/blocks/canvas/ew-editor-doc/utils/selection.js +++ b/blocks/canvas/ew-editor-doc/utils/selection.js @@ -68,13 +68,10 @@ export function selectedNodePayload(view) { return null; } -// Mirrors the outline's own proseIndex convention (getDefaultContentProseIndex / -// data-prose-index), which is NOT simply each node's own start — non-image content is -// indexed one position *inside* its own start (posAtDOM(el, 0)), because da-nx's -// inline-editor bootstrap depends on that exact value as its cursorOffset (see -// prose-diff.js/da-nx's prose.js). Images are the one exception: data-image-index stores -// the node's own start directly, since an atomic node has no "inside". Blocks (table) -// are excluded entirely — they're tracked via blockIndex, never as outline content-children. +// Mirrors data-prose-index/getDefaultContentProseIndex: non-image content is indexed +// one position *inside* its own start (posAtDOM(el, 0)), since da-nx's inline-editor +// bootstrap depends on that exact value as its cursorOffset (see prose-diff.js/da-nx's +// prose.js). Images use their own start directly; tables are excluded (tracked via blockIndex). export function activeContentProseIndex(view) { const sel = view?.state?.selection; if (!sel) return undefined; diff --git a/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js b/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js index 2f1e1e9bf..4fe644e1d 100644 --- a/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js +++ b/blocks/canvas/ew-editor-wysiwyg/utils/handlers.js @@ -57,10 +57,9 @@ export function handleCursorMove({ cursorOffset, textCursorOffset }, ctx) { } ctx.suppressRerender = true; - // dispatch() synchronously runs createTrackingPlugin's view-update hook, which already - // emits editorSelectChange with the full { blockIndex, proseIndex, source: 'doc' } - // payload for this exact selection — a second, blockIndex-only emit here would - // clobber that (dropping proseIndex) and incorrectly collapse the outline. + // dispatch() already triggers createTrackingPlugin's hook, which emits editorSelectChange + // with the full payload (incl. proseIndex) — a second, blockIndex-only emit here would + // clobber that and collapse the outline. view.dispatch(tr.scrollIntoView()); ctx.suppressRerender = false; const tb = getSelectionToolbar(); diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index 813eb6da2..e166aed7b 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -165,9 +165,8 @@ class EwPageOutline extends LitElement { this._expandedContent = next; } - // Selection changes always fully reset expansion (see _renderContentGroup): a block - // selection collapses every run, a content selection expands only its own run. Manual - // expand/collapse (_toggleContentGroup) persists only until the next selection change. + // A block selection collapses every run; a content selection expands only its own + // (see _renderContentGroup). Manual toggles (_toggleContentGroup) survive until then. _resetExpansionForBlock() { this._expandedContent = new Set(); } diff --git a/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js b/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js index eb1f1b0af..257395d23 100644 --- a/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js +++ b/test/unit/blocks/canvas/ew-editor-doc/ew-editor-doc.test.js @@ -10,9 +10,8 @@ before(async () => { await import('../../../../../blocks/canvas/ew-editor-doc/ew-editor-doc.js'); }); -// Wraps view.dispatch so tests can assert whether the guarded early-returns in -// _scrollDocToProseIndex actually skip dispatching, while still letting dispatched -// transactions apply so the resulting selection can be inspected. +// Wraps view.dispatch so tests can assert the guarded early-returns in +// _scrollDocToProseIndex skip dispatching, while still applying transactions that do. function spyDispatch(view) { const calls = []; const original = view.dispatch.bind(view); @@ -23,8 +22,7 @@ function spyDispatch(view) { return calls; } -// Replaces the default single-paragraph doc with a text paragraph followed by a -// paragraph wrapping an image, mirroring how a real page mixes prose and images. +// Replaces the default doc with a text paragraph + an image paragraph, mirroring a real page. function buildDoc(view) { const { schema } = view.state; const textPara = schema.nodes.paragraph.create(null, schema.text('hello world')); From c6c94132edda481ffd37937cdd7835ed17f57865 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 12:00:54 -0700 Subject: [PATCH 17/21] fix(canvas): keep prose2aem free of canvas-only empty-paragraph handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the keepEmptyParagraphs flag added to the shared prose2aem module (used by save/publish/live-preview/diff) and moves empty-node handling entirely into canvas's getInstrumentedHTML. Emptiness is now determined from the actual ProseMirror node (content.size === 0) rather than a flag threaded through shared code, and empty

        s are masked with a placeholder comment before serialization, then stripped back out — so the outline can still see/select an empty node being edited, while SET_BODY to the layout view and all other prose2aem callers are unaffected. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/editor-utils.js | 20 ++++++++++++++++---- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 12 +++++++----- blocks/shared/prose2aem.js | 16 ++++------------ 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 2e4ab584d..6f534131b 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -176,7 +176,9 @@ export function extractCursors(view) { return [...cursorMap.values()]; } -export function getInstrumentedHTML(view) { +const EMPTY_PARAGRAPH_MARKER = 'da-canvas-empty-paragraph'; + +export function getInstrumentedHTML(view, { keepEmptyParagraphs = false } = {}) { const editorClone = view.dom.cloneNode(true); const originalElements = view.dom.querySelectorAll(EDITABLE_SELECTORS); @@ -188,6 +190,15 @@ export function getInstrumentedHTML(view) { try { const editableElementStartPos = view.posAtDOM(originalElement, 0); clonedElements[index].setAttribute('data-prose-index', editableElementStartPos); + // prose2aem strips empty

        tags on save/publish. Mask truly-empty ones (per the + // PM doc itself, not DOM decorations like cursors/gap-cursor) so this copy's

        + // survives serialization for the outline to see and select. Unmasked below. + if (keepEmptyParagraphs && originalElement.tagName === 'P') { + const node = view.state.doc.resolve(editableElementStartPos).parent; + if (node.type.name === 'paragraph' && node.content.size === 0) { + clonedElements[index].append(document.createComment(EMPTY_PARAGRAPH_MARKER)); + } + } } catch (e) { // eslint-disable-next-line no-console console.warn('Could not find position for element:', e); @@ -253,9 +264,10 @@ export function getInstrumentedHTML(view) { // Serialize clone to HTML, then move block-marker index onto wrapper as data-block-index // (same pattern as da-nx qe-advanced: getInstrumentedHTML in prose2aem.js). - // keepEmptyParagraphs: an empty

        being actively edited must survive here so - // parseSections/the outline can see and select it; the saved/published HTML still strips it. - let htmlString = prose2aem(editorClone, true, false, true); + let htmlString = prose2aem(editorClone, true, false); + if (keepEmptyParagraphs) { + htmlString = htmlString.replaceAll(``, ''); + } htmlString = htmlString.replace( /

        <\/div>\s*]*?)>/gi, (_match, proseIndex, divAttributes) => ``, diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index 8cefc0249..a09435505 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -87,7 +87,7 @@ export class EwEditorDoc extends LitElement { _emitHtmlChange() { const { view } = this._proseContext ?? {}; if (!view) return; - editorHtmlChange.emit(getInstrumentedHTML(view)); + editorHtmlChange.emit(getInstrumentedHTML(view, { keepEmptyParagraphs: true })); } _emitUndoState() { @@ -283,10 +283,12 @@ export class EwEditorDoc extends LitElement { createExtensionsBridgePlugin(), createTrackingPlugin( () => { - const body = this._controllerCtx - ? updateDocument(this._controllerCtx) - : getInstrumentedHTML(this._proseContext?.view); - if (body) editorHtmlChange.emit(body); + // SET_BODY (layout view/live preview) strips empty paragraphs; the outline's + // copy keeps them so an empty node being edited stays visible/selectable. + if (this._controllerCtx) updateDocument(this._controllerCtx); + const docView = this._proseContext?.view; + if (!docView) return; + editorHtmlChange.emit(getInstrumentedHTML(docView, { keepEmptyParagraphs: true })); }, () => { if (this._controllerCtx) updateCursors(this._controllerCtx); }, (data) => { if (this._controllerCtx) getEditor(data, this._controllerCtx); }, diff --git a/blocks/shared/prose2aem.js b/blocks/shared/prose2aem.js index 6d6d949a7..4e0aa2331 100644 --- a/blocks/shared/prose2aem.js +++ b/blocks/shared/prose2aem.js @@ -134,11 +134,11 @@ function makePictures(editor, live) { }); } -function convertParagraphs(editor, keepEmpty = false) { +function convertParagraphs(editor) { const paras = editor.querySelectorAll(':scope > p'); paras.forEach((p) => { // Remove empty p tags - if (!keepEmpty && p.innerHTML.trim() === '') { p.remove(); } + if (p.innerHTML.trim() === '') { p.remove(); } // Convert dash p tags to rules if (p.textContent.trim() === '---') { const hr = document.createElement('hr'); @@ -263,17 +263,9 @@ function convertLocalUrlsToRelative(editor) { * @param {HTMLElement} editor the editor dom * @param {Boolean} livePreview whether or not the target destination is Live Preview * @param {Boolean} isFragment whether or not the DOM is a fragment - * @param {Boolean} keepEmptyParagraphs keep empty

        tags instead of stripping them - * (canvas's instrumented HTML needs these so an empty node being edited stays - * visible/selectable in the outline; the actual saved/published HTML still strips them) * @returns AEM-friendly HTML as a text string */ -export default function prose2aem( - editor, - livePreview, - isFragment = false, - keepEmptyParagraphs = false, -) { +export default function prose2aem(editor, livePreview, isFragment = false) { if (!isFragment) editor.removeAttribute('class'); editor.removeAttribute('contenteditable'); @@ -305,7 +297,7 @@ export default function prose2aem( convertListItems(editor); - convertParagraphs(editor, keepEmptyParagraphs); + convertParagraphs(editor); convertBlocks(editor, isFragment); From 02e6522fcb35333186431e7cd5e47a5cdb735102 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 15:10:33 -0700 Subject: [PATCH 18/21] fix(canvas): remove empty-node highlight/select behavior from outline Reverts the keepEmptyParagraphs plumbing and outline visibility logic that kept empty paragraphs/headings alive through instrumented HTML so they could be seen and selected in the outline. Too much complexity for the value; empty nodes are skipped again when building content runs, same as before PR #1167 introduced the feature. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/editor-utils.js | 29 ++---- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 12 +-- .../canvas/ew-page-outline/ew-page-outline.js | 9 +- .../canvas/editor-utils/editor-utils.test.js | 99 ++----------------- 4 files changed, 22 insertions(+), 127 deletions(-) diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 6f534131b..80367b33b 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -176,9 +176,7 @@ export function extractCursors(view) { return [...cursorMap.values()]; } -const EMPTY_PARAGRAPH_MARKER = 'da-canvas-empty-paragraph'; - -export function getInstrumentedHTML(view, { keepEmptyParagraphs = false } = {}) { +export function getInstrumentedHTML(view) { const editorClone = view.dom.cloneNode(true); const originalElements = view.dom.querySelectorAll(EDITABLE_SELECTORS); @@ -190,15 +188,6 @@ export function getInstrumentedHTML(view, { keepEmptyParagraphs = false } = {}) try { const editableElementStartPos = view.posAtDOM(originalElement, 0); clonedElements[index].setAttribute('data-prose-index', editableElementStartPos); - // prose2aem strips empty

        tags on save/publish. Mask truly-empty ones (per the - // PM doc itself, not DOM decorations like cursors/gap-cursor) so this copy's

        - // survives serialization for the outline to see and select. Unmasked below. - if (keepEmptyParagraphs && originalElement.tagName === 'P') { - const node = view.state.doc.resolve(editableElementStartPos).parent; - if (node.type.name === 'paragraph' && node.content.size === 0) { - clonedElements[index].append(document.createComment(EMPTY_PARAGRAPH_MARKER)); - } - } } catch (e) { // eslint-disable-next-line no-console console.warn('Could not find position for element:', e); @@ -265,9 +254,6 @@ export function getInstrumentedHTML(view, { keepEmptyParagraphs = false } = {}) // Serialize clone to HTML, then move block-marker index onto wrapper as data-block-index // (same pattern as da-nx qe-advanced: getInstrumentedHTML in prose2aem.js). let htmlString = prose2aem(editorClone, true, false); - if (keepEmptyParagraphs) { - htmlString = htmlString.replaceAll(``, ''); - } htmlString = htmlString.replace( /

        <\/div>\s*]*?)>/gi, (_match, proseIndex, divAttributes) => ``, @@ -317,11 +303,9 @@ function getDefaultContentKind(el) { if (tag === 'UL') return { kind: 'list', ordered: false }; if (tag === 'PRE') return { kind: 'code' }; if (tag === 'BLOCKQUOTE') return { kind: 'quote' }; - if (el.textContent?.trim()) return { kind: 'paragraph' }; - // A text-less

        wraps only an image, as does a bare / — but a text-less - //

        with no image at all is just an empty paragraph, not an image wrapper. - const hasImage = el.matches?.('img') || !!el.querySelector?.('img'); - return { kind: hasImage ? 'image' : 'paragraph' }; + // a text-less

        wraps only an image, as does a bare /; + // anything with text is a paragraph + return { kind: el.textContent?.trim() ? 'paragraph' : 'image' }; } export function parseSections(htmlText) { @@ -347,7 +331,6 @@ export function parseSections(htmlText) { proseIndex: getDefaultContentProseIndex(el, kindInfo.kind), innerText: el.textContent.trim(), snippet: getContentSnippet(el, kindInfo.kind), - empty: !hasDefaultContent(el), }; }), }); @@ -371,7 +354,9 @@ export function parseSections(htmlText) { return; } - currentRun.push(el); + // Skip empty nodes — prose2aem doesn't always strip them (e.g. an empty

        can + // survive serialization) — so they neither break nor join a run. + if (hasDefaultContent(el)) currentRun.push(el); }); flushRun(); diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index a09435505..8cefc0249 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -87,7 +87,7 @@ export class EwEditorDoc extends LitElement { _emitHtmlChange() { const { view } = this._proseContext ?? {}; if (!view) return; - editorHtmlChange.emit(getInstrumentedHTML(view, { keepEmptyParagraphs: true })); + editorHtmlChange.emit(getInstrumentedHTML(view)); } _emitUndoState() { @@ -283,12 +283,10 @@ export class EwEditorDoc extends LitElement { createExtensionsBridgePlugin(), createTrackingPlugin( () => { - // SET_BODY (layout view/live preview) strips empty paragraphs; the outline's - // copy keeps them so an empty node being edited stays visible/selectable. - if (this._controllerCtx) updateDocument(this._controllerCtx); - const docView = this._proseContext?.view; - if (!docView) return; - editorHtmlChange.emit(getInstrumentedHTML(docView, { keepEmptyParagraphs: true })); + const body = this._controllerCtx + ? updateDocument(this._controllerCtx) + : getInstrumentedHTML(this._proseContext?.view); + if (body) editorHtmlChange.emit(body); }, () => { if (this._controllerCtx) updateCursors(this._controllerCtx); }, (data) => { if (this._controllerCtx) getEditor(data, this._controllerCtx); }, diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index e166aed7b..d32b8b7ac 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -37,8 +37,7 @@ const DROP_POSITIONS = { function contentChildEqual(child, other) { return child.proseIndex === other.proseIndex && child.innerText === other.innerText - && child.kind === other.kind && child.level === other.level && child.ordered === other.ordered - && child.empty === other.empty; + && child.kind === other.kind && child.level === other.level && child.ordered === other.ordered; } function contentChildLabel(child) { @@ -394,10 +393,6 @@ class EwPageOutline extends LitElement { } _renderContentGroup(item, isFirst) { - const visibleChildren = item.children.filter( - (child) => !child.empty || child.proseIndex === this._selectedProseIndex, - ); - if (!visibleChildren.length) return nothing; const key = item.proseIndex; const expanded = this._expandedContent?.has(key); return html` @@ -412,7 +407,7 @@ class EwPageOutline extends LitElement {

        ${expanded ? html`
          - ${visibleChildren.map((child) => html` + ${item.children.map((child) => html`
        • { type: 'content', proseIndex: 1, innerText: 'Intro text', - children: [{ - type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Intro text', snippet: 'Intro text', empty: false, - }], + children: [{ type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Intro text', snippet: 'Intro text' }], }); expect(section.items[2]).to.deep.equal({ type: 'content', proseIndex: 20, innerText: 'Outro text', - children: [{ - type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Outro text', snippet: 'Outro text', empty: false, - }], + children: [{ type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Outro text', snippet: 'Outro text' }], }); }); @@ -182,14 +178,10 @@ describe('parseSections', () => { innerText: 'Title Para one Para two', children: [ { - type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title', snippet: 'Title', empty: false, - }, - { - type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one', snippet: 'Para one', empty: false, - }, - { - type: 'content', kind: 'paragraph', proseIndex: 12, innerText: 'Para two', snippet: 'Para two', empty: false, + type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: 'Title', snippet: 'Title', }, + { type: 'content', kind: 'paragraph', proseIndex: 5, innerText: 'Para one', snippet: 'Para one' }, + { type: 'content', kind: 'paragraph', proseIndex: 12, innerText: 'Para two', snippet: 'Para two' }, ], }]); }); @@ -248,53 +240,7 @@ describe('parseSections', () => { const [section] = parseSections(html); const [{ children }] = section.items; expect(children.map((c) => c.kind)).to.deep.equal(['heading', 'code']); - expect(children[1]).to.deep.equal({ - type: 'content', kind: 'code', proseIndex: 5, innerText: 'const x = 1;', snippet: 'const x = 1;', empty: false, - }); - }); - - it('keeps empty loose nodes in the run, flagged empty, without breaking it', () => { - // getInstrumentedHTML stamps data-prose-index on every editable element, empty or - // not (keepEmptyParagraphs keeps the empty

          itself alive through prose2aem too). - const html = `

          -

          Para one

          -

          -

          -

          Para two

          -
          `; - const [section] = parseSections(html); - expect(section.items).to.deep.equal([{ - type: 'content', - proseIndex: 1, - innerText: 'Para one Para two', - children: [ - { - type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Para one', snippet: 'Para one', empty: false, - }, - { - type: 'content', kind: 'heading', level: 2, proseIndex: 8, innerText: '', snippet: '', empty: true, - }, - { - type: 'content', kind: 'paragraph', proseIndex: 12, innerText: '', snippet: '', empty: true, - }, - { - type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Para two', snippet: 'Para two', empty: false, - }, - ], - }]); - }); - - it('produces a content entry flagged empty for a run made up entirely of empty nodes', () => { - const html = `
          -
          Hero
          -

          -

          -
          `; - const [section] = parseSections(html); - expect(section.items).to.have.length(2); - expect(section.items[0].type).to.equal('block'); - expect(section.items[1].type).to.equal('content'); - expect(section.items[1].children.every((c) => c.empty)).to.be.true; + expect(children[1]).to.deep.equal({ type: 'content', kind: 'code', proseIndex: 5, innerText: 'const x = 1;', snippet: 'const x = 1;' }); }); it('reads proseIndex from data-image-index on a loose image', () => { @@ -306,34 +252,7 @@ describe('parseSections', () => { type: 'content', proseIndex: 7, innerText: '', - children: [{ - type: 'content', kind: 'image', proseIndex: 7, innerText: '', snippet: '', empty: false, - }], - }]); - }); - - it('takes run identity from the first node in the run, even when that node is empty', () => { - const html = `
          -

          -

          First real content

          -

          More content

          -
          `; - const [section] = parseSections(html); - expect(section.items).to.deep.equal([{ - type: 'content', - proseIndex: 1, - innerText: 'First real content More content', - children: [ - { - type: 'content', kind: 'heading', level: 2, proseIndex: 1, innerText: '', snippet: '', empty: true, - }, - { - type: 'content', kind: 'paragraph', proseIndex: 9, innerText: 'First real content', snippet: 'First real content', empty: false, - }, - { - type: 'content', kind: 'paragraph', proseIndex: 15, innerText: 'More content', snippet: 'More content', empty: false, - }, - ], + children: [{ type: 'content', kind: 'image', proseIndex: 7, innerText: '', snippet: '' }], }]); }); @@ -348,9 +267,7 @@ describe('parseSections', () => { type: 'content', proseIndex: 1, innerText: 'Section one text', - children: [{ - type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Section one text', snippet: 'Section one text', empty: false, - }], + children: [{ type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'Section one text', snippet: 'Section one text' }], }]); expect(sections[1].items).to.deep.equal([ { type: 'block', name: 'cards', blockIndex: 0, proseIndex: 0, innerText: 'Cards' }, From a83584ab3fc1854f6e01354996bfb97a8a34736f Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 15:39:20 -0700 Subject: [PATCH 19/21] feat(canvas): simplify outline expansion state; keep runs open across selection/delete Replaces the per-selection collapse/expand-only-one-run model with a simpler one: expansion only resets on a real reparse (a structural edit that changes _sections); selection changes only additively expand the run holding the new selection, never collapse anything else. A range-based fallback in _findRunKeyForProseIndex also expands a run when the selection lands on a node that has no row of its own (e.g. a fresh empty paragraph from pressing Enter, filtered out of parseSections), not just an exact child match. Also drops the now-vestigial sibling-select-on-delete workaround in deleteContentItem (it stopped being what kept a run visible once expansion became reparse-driven) and replaces it with a direct fix: _onDelete captures the deleted content item's run by array position and re-expands it after the delete's reparse, so deleting a child no longer collapses its own run. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/blocks.js | 25 +-- .../canvas/ew-page-outline/ew-page-outline.js | 66 +++++--- .../blocks/canvas/editor-utils/blocks.test.js | 40 +---- .../ew-page-outline/ew-page-outline.test.js | 145 ++++++++++++++++-- 4 files changed, 188 insertions(+), 88 deletions(-) diff --git a/blocks/canvas/editor-utils/blocks.js b/blocks/canvas/editor-utils/blocks.js index 67fd2f634..608998742 100644 --- a/blocks/canvas/editor-utils/blocks.js +++ b/blocks/canvas/editor-utils/blocks.js @@ -101,32 +101,11 @@ export function getContentItemRange(doc, child) { return node ? { pos, size: node.nodeSize, node } : null; } -function isContentNode(node, schema) { - return node.type.name !== 'table' && node.type !== schema.nodes.horizontal_rule; -} - -function getRunSibling(doc, schema, pos, direction) { - const topNodes = []; - doc.forEach((node, nodePos) => topNodes.push({ node, pos: nodePos })); - const idx = topNodes.findIndex((entry) => entry.pos === pos); - if (idx === -1) return null; - const sibling = topNodes[idx + direction]; - if (!sibling || !isContentNode(sibling.node, schema)) return null; - return sibling; -} - export function deleteContentItem(view, child) { if (!view) return; - const { doc, schema } = view.state; - const range = getContentItemRange(doc, child); + const range = getContentItemRange(view.state.doc, child); if (!range) return; - - const sibling = getRunSibling(doc, schema, range.pos, 1) - ?? getRunSibling(doc, schema, range.pos, -1); - - const tr = view.state.tr.delete(range.pos, range.pos + range.size); - if (sibling) tr.setSelection(NodeSelection.create(tr.doc, tr.mapping.map(sibling.pos))); - view.dispatch(tr); + view.dispatch(view.state.tr.delete(range.pos, range.pos + range.size)); } function getSectionStartOffset(view, sectionIndex) { diff --git a/blocks/canvas/ew-page-outline/ew-page-outline.js b/blocks/canvas/ew-page-outline/ew-page-outline.js index d32b8b7ac..f58194530 100644 --- a/blocks/canvas/ew-page-outline/ew-page-outline.js +++ b/blocks/canvas/ew-page-outline/ew-page-outline.js @@ -89,7 +89,13 @@ class EwPageOutline extends LitElement { this._unsubscribeHtml = editorHtmlChange.subscribe((aemHtml) => { if (aemHtml.trim()) { const next = parseSections(aemHtml); - if (!sectionsEqual(next, this._sections)) this._sections = next; + if (!sectionsEqual(next, this._sections)) { + this._sections = next; + // A structural edit is the only time proseIndex-keyed expansion state can go + // stale (positions shift), so this is the one point where it's safe to drop — + // selection changes never do (see _expandRunForProse). + this._expandedContent = new Set(); + } } else { this._sections = undefined; this._selectedBlockIndex = undefined; @@ -101,9 +107,7 @@ class EwPageOutline extends LitElement { if (source === 'outline') return; this._selectedBlockIndex = blockIndex; this._selectedProseIndex = proseIndex; - if (blockIndex != null && blockIndex >= 0) this._resetExpansionForBlock(); - else if (proseIndex != null) this._resetExpansionForProse(proseIndex); - else this._resetExpansionForBlock(); + if (proseIndex != null) this._expandRunForProse(proseIndex); }); } @@ -146,14 +150,13 @@ class EwPageOutline extends LitElement { _select(blockIndex) { this._selectedBlockIndex = blockIndex; this._selectedProseIndex = undefined; - this._resetExpansionForBlock(); editorSelectChange.emit({ blockIndex, source: 'outline' }); } _selectProse(proseIndex, kind) { this._selectedProseIndex = proseIndex; this._selectedBlockIndex = undefined; - this._resetExpansionForProse(proseIndex); + this._expandRunForProse(proseIndex); editorProseSelectChange.emit({ proseIndex, kind }); } @@ -164,22 +167,30 @@ class EwPageOutline extends LitElement { this._expandedContent = next; } - // A block selection collapses every run; a content selection expands only its own - // (see _renderContentGroup). Manual toggles (_toggleContentGroup) survive until then. - _resetExpansionForBlock() { - this._expandedContent = new Set(); - } - - _resetExpansionForProse(proseIndex) { + // Selection never collapses anything — it only ensures the run holding the new + // selection is visible, adding it alongside whatever's already expanded. Expansion is + // only ever cleared wholesale on a reparse (see the editorHtmlChange subscription). + _expandRunForProse(proseIndex) { const runKey = this._findRunKeyForProseIndex(proseIndex); - this._expandedContent = runKey != null ? new Set([runKey]) : new Set(); + if (runKey == null) return; + this._expandedContent = new Set(this._expandedContent).add(runKey); } _findRunKeyForProseIndex(proseIndex) { - for (const sec of this._sections ?? []) { - for (const item of sec.items) { - if (item.type === 'content' && item.children.some((child) => child.proseIndex === proseIndex)) { - return item.proseIndex; + const sections = this._sections ?? []; + for (const [secIdx, sec] of sections.entries()) { + for (const [itemIdx, item] of sec.items.entries()) { + if (item.type === 'content') { + const hasChild = item.children.some((child) => child.proseIndex === proseIndex); + if (hasChild) return item.proseIndex; + + // A node invisible in the outline (e.g. a fresh empty paragraph from pressing + // Enter) still belongs to this run if its position falls between the run's own + // start and whatever comes next — the next item in this section, the next + // section's first item, or unbounded if this is the very last item overall. + const nextItem = sec.items[itemIdx + 1] ?? sections[secIdx + 1]?.items[0]; + const upperBound = nextItem ? nextItem.proseIndex : Infinity; + if (proseIndex >= item.proseIndex && proseIndex < upperBound) return item.proseIndex; } } } @@ -362,6 +373,19 @@ class EwPageOutline extends LitElement { openBlockLibraryModal({ onInsert }); } + // Array position (not proseIndex) of the run holding this child, so a delete can find + // it again afterward without comparing positions across the edit (see _onDelete). + _findRunLocation(proseIndex) { + for (const [sectionIndex, sec] of (this._sections ?? []).entries()) { + const itemIndex = sec.items.findIndex( + (item) => item.type === 'content' + && item.children.some((child) => child.proseIndex === proseIndex), + ); + if (itemIndex !== -1) return { sectionIndex, itemIndex }; + } + return undefined; + } + _onDelete(e, type, index) { e.stopPropagation(); e.preventDefault(); @@ -370,7 +394,13 @@ class EwPageOutline extends LitElement { if (type === OUTLINE_TYPES.BLOCK) { deleteBlock(view, index); } else if (type === OUTLINE_TYPES.CONTENT) { + const location = this._findRunLocation(index.proseIndex); deleteContentItem(view, index); + // A content-child delete never reorders/merges runs, only shrinks or removes the + // deleted-from one — so the same array position still identifies it, if it survived. + const survivingRun = location + && this._sections?.[location.sectionIndex]?.items[location.itemIndex]; + if (survivingRun?.type === 'content') this._expandRunForProse(survivingRun.proseIndex); } else { deleteSection(view, index); } diff --git a/test/unit/blocks/canvas/editor-utils/blocks.test.js b/test/unit/blocks/canvas/editor-utils/blocks.test.js index 411008514..736218206 100644 --- a/test/unit/blocks/canvas/editor-utils/blocks.test.js +++ b/test/unit/blocks/canvas/editor-utils/blocks.test.js @@ -148,7 +148,7 @@ describe('deleteContentItem', () => { expect(view.state.doc.firstChild.textContent).to.equal('Keep me'); }); - it('selects the next sibling in the same run after deleting a middle child', () => { + it('leaves selection to PM\'s own mapping rather than selecting a sibling (matches deleteBlock)', () => { const view = makeRealView({ type: 'doc', content: [ @@ -161,45 +161,9 @@ describe('deleteContentItem', () => { deleteContentItem(view, child); const { selection, doc } = view.state; - expect(selection).to.be.instanceOf(NodeSelection); - expect(selection.node.textContent).to.equal('C'); + expect(selection).to.not.be.instanceOf(NodeSelection); expect(doc.childCount).to.equal(2); }); - - it('falls back to the previous sibling when the deleted child was last in its run', () => { - const view = makeRealView({ - type: 'doc', - content: [ - { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, - { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, - { type: 'paragraph', content: [{ type: 'text', text: 'C' }] }, - ], - }); - const child = childrenOf(view).find((c) => c.innerText === 'C'); - deleteContentItem(view, child); - - const { selection } = view.state; - expect(selection).to.be.instanceOf(NodeSelection); - expect(selection.node.textContent).to.equal('B'); - }); - - it('never crosses a block boundary to find a sibling to select', () => { - const view = makeRealView({ - type: 'doc', - content: [ - tableJSON('hero'), - { type: 'paragraph', content: [{ type: 'text', text: 'Lone para' }] }, - tableJSON('cards'), - ], - }); - const child = childrenOf(view).find((c) => c.innerText === 'Lone para'); - deleteContentItem(view, child); - - // No same-run sibling exists (blocks on both sides) — nothing gets deliberately - // selected, so the neighboring tables are never turned into a NodeSelection. - expect(view.state.selection).to.not.be.instanceOf(NodeSelection); - expect(view.state.doc.childCount).to.equal(2); - }); }); describe('moveContentItem', () => { diff --git a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js index 5549b012c..53c5c83be 100644 --- a/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js +++ b/test/unit/blocks/canvas/ew-page-outline/ew-page-outline.test.js @@ -1,10 +1,14 @@ /* eslint-disable no-underscore-dangle */ import { expect } from '@esm-bundle/chai'; +import { EditorState, EditorView } from 'da-y-wrapper'; +import { getSchema } from 'da-parser'; import { setNx } from '../../../../../scripts/utils.js'; import { makeRealView } from '../test-helpers.js'; +import { createTrackingPlugin } from '../../../../../blocks/canvas/editor-utils/prose-diff.js'; setNx('/test/fixtures/nx', { hostname: 'example.com' }); +let editorHtmlChange; let editorProseSelectChange; let editorSelectChange; let getExtensionsBridge; @@ -13,7 +17,9 @@ let parseSections; before(async () => { await import('../../../../../blocks/canvas/ew-page-outline/ew-page-outline.js'); - ({ editorProseSelectChange, editorSelectChange, getInstrumentedHTML, parseSections } = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js')); + const editorUtils = await import('../../../../../blocks/canvas/editor-utils/editor-utils.js'); + ({ editorHtmlChange, editorProseSelectChange, editorSelectChange } = editorUtils); + ({ getInstrumentedHTML, parseSections } = editorUtils); ({ getExtensionsBridge } = await import('../../../../../blocks/canvas/editor-utils/extensions-bridge.js')); }); @@ -30,6 +36,21 @@ function docSeq(doc) { return seq; } +// Unlike makeRealView, wires the real tracking plugin so a delete/insert dispatched +// through it auto-emits editorHtmlChange exactly like the production editor does — +// needed to test that a reparse-driven expansion reset/re-expand actually happens. +function makeTrackedView(json) { + const schema = getSchema(); + const doc = schema.nodeFromJSON(json); + const dom = document.createElement('div'); + document.body.appendChild(dom); + let view; + const plugins = [createTrackingPlugin(() => editorHtmlChange.emit(getInstrumentedHTML(view)))]; + const state = EditorState.create({ schema, doc, plugins }); + view = new EditorView(dom, { state }); + return view; +} + async function createOutline() { const el = document.createElement('ew-page-outline'); // _checkBlockLibrary fires once a hash with org/site is set — no-op it so this @@ -128,7 +149,7 @@ describe('ew-page-outline — expandable default content', () => { expect(received).to.deep.equal({ proseIndex: 9, kind: 'image' }); }); - it('marks a clicked content child as selected, and collapses its run when a block is selected instead', async () => { + it('marks a clicked content child as selected, and leaves its run expanded when a block is selected instead', async () => { el._sections[0].blocks = [{ name: 'hero', blockIndex: 0 }]; el.shadowRoot.querySelector('.content-item').click(); await el.updateComplete; @@ -143,17 +164,18 @@ describe('ew-page-outline — expandable default content', () => { el._select(0); await el.updateComplete; - // Selecting a block resets expansion, collapsing every run — the previously - // selected child is no longer rendered at all, not just unmarked as selected. - expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('false'); - expect(el.shadowRoot.querySelectorAll('.content-child')).to.have.lengthOf(0); + // Selecting a block no longer collapses anything — the run stays expanded and its + // children stay rendered, just no longer marked selected. + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + expect(paragraphChild.classList.contains('selected')).to.be.false; + expect(el.shadowRoot.querySelectorAll('.content-child')).to.have.lengthOf(5); }); - it('highlights a content child when the doc selection (not just an outline click) lands on it, and collapses its run on block selection', async () => { + it('highlights a content child when the doc selection (not just an outline click) lands on it, and leaves it expanded on a later block selection', async () => { editorSelectChange.emit({ blockIndex: -1, proseIndex: 5, source: 'doc' }); await el.updateComplete; - // A content selection expands only the run containing it, with no manual click needed. + // A collapsed run expands additively to reveal a new selection, with no manual click needed. expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); const paragraphChild = [...el.shadowRoot.querySelectorAll('.content-child')][1]; expect(paragraphChild.classList.contains('selected')).to.be.true; @@ -161,8 +183,88 @@ describe('ew-page-outline — expandable default content', () => { editorSelectChange.emit({ blockIndex: 0, proseIndex: undefined, source: 'doc' }); await el.updateComplete; + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + expect(paragraphChild.classList.contains('selected')).to.be.false; + expect(el.shadowRoot.querySelectorAll('.content-child')).to.have.lengthOf(5); + }); + + it('expands a run when the selection lands between two of its children, not on one exactly', async () => { + // Simulates pressing Enter mid-paragraph: the new empty node has no row of its own + // (filtered out of parseSections), but its proseIndex (7) falls between the + // surrounding real children (5 and 9), so it should still resolve to their run. + editorSelectChange.emit({ blockIndex: -1, proseIndex: 7, source: 'doc' }); + await el.updateComplete; + + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + }); + + it('does not let an unmatched proseIndex leak expansion into a neighboring run', async () => { + el._sections = [ + { + sectionIndex: 0, + blocks: [], + items: [contentGroupItem(1, [ + { type: 'content', kind: 'paragraph', proseIndex: 1, innerText: 'One', snippet: 'One' }, + ])], + }, + { + sectionIndex: 1, + blocks: [], + items: [contentGroupItem(20, [ + { type: 'content', kind: 'paragraph', proseIndex: 20, innerText: 'Two', snippet: 'Two' }, + ])], + }, + ]; + await el.updateComplete; + + // Group headers only — `.content-item` also matches rendered content-child rows. + const headers = () => el.shadowRoot.querySelectorAll('.content-group > .content-item'); + + // proseIndex 10 sits after section 0's only child (1) but well before section 1's + // (20) — with no next item in section 0 to bound it, it's attributed to section 0's + // run (the trailing/unbounded case a fresh Enter-created node at the end lands in). + editorSelectChange.emit({ blockIndex: -1, proseIndex: 10, source: 'doc' }); + await el.updateComplete; + + expect(headers()[0].getAttribute('aria-expanded')).to.equal('true'); + expect(headers()[1].getAttribute('aria-expanded')).to.equal('false'); + + // proseIndex 25, past section 1's only child with nothing after it, resolves there. + editorSelectChange.emit({ blockIndex: -1, proseIndex: 25, source: 'doc' }); + await el.updateComplete; + + expect(headers()[1].getAttribute('aria-expanded')).to.equal('true'); + }); + + it('resets expansion only when a structural edit actually changes the sections', async () => { + // Drives _sections through the real editorHtmlChange/parseSections pipeline (rather + // than the manual fixture in beforeEach) so re-emitting identical HTML is guaranteed + // to parse to a sectionsEqual result. + const initialHtml = `
          +

          Title

          +

          Para one

          +
          `; + editorHtmlChange.emit(initialHtml); + await el.updateComplete; + + el.shadowRoot.querySelector('.content-item').click(); + await el.updateComplete; + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + + editorHtmlChange.emit(initialHtml); + await el.updateComplete; + + // Same HTML reparses to an equal section tree — sectionsEqual holds, expansion survives. + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + + const changedHtml = `
          +

          Title

          +
          `; + editorHtmlChange.emit(changedHtml); + await el.updateComplete; + + // Structural change (a child removed) — sectionsEqual fails, expansion resets. expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('false'); - expect(el.shadowRoot.querySelectorAll('.content-child')).to.have.lengthOf(0); }); it('expands and collapses the focused group header with ArrowRight/ArrowLeft', async () => { @@ -219,6 +321,31 @@ describe('ew-page-outline — content drag & delete', () => { expect(docSeq(bridge.view.state.doc)).to.deep.equal(['Keep me']); }); + it('keeps a run expanded after deleting one of its children', async () => { + bridge.view = makeTrackedView({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'A' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'B' }] }, + ], + }); + + editorHtmlChange.emit(getInstrumentedHTML(bridge.view)); + await el.updateComplete; + + el.shadowRoot.querySelector('.content-item').click(); + await el.updateComplete; + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + + el.shadowRoot.querySelectorAll('.content-child .delete-btn')[0].click(); + await el.updateComplete; + + expect(docSeq(bridge.view.state.doc)).to.deep.equal(['B']); + // No sibling-select workaround needed — the run survived the reparse-driven reset + // because _onDelete re-expands it by array position (see _findRunLocation). + expect(el.shadowRoot.querySelector('.content-item').getAttribute('aria-expanded')).to.equal('true'); + }); + it('reorders content children via drop onto another content child', () => { bridge.view = makeRealView({ type: 'doc', From a42c94d93cc671c822ad31e6bfd30d0c18e0568a Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 15:53:56 -0700 Subject: [PATCH 20/21] fix(canvas): restore image detection in getDefaultContentKind, drop dead prose2aem arg getDefaultContentKind now explicitly checks for a nested/matching before falling back to 'paragraph', instead of assuming a text-less element is always an image. Also reverts the getInstrumentedHTML->prose2aem call's trailing arg back to what main already has (unused since before this PR, prose2aem never read a 4th param) so that line doesn't show up as an unnecessary diff. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/editor-utils/editor-utils.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/blocks/canvas/editor-utils/editor-utils.js b/blocks/canvas/editor-utils/editor-utils.js index 80367b33b..6b7ca0225 100644 --- a/blocks/canvas/editor-utils/editor-utils.js +++ b/blocks/canvas/editor-utils/editor-utils.js @@ -253,7 +253,7 @@ export function getInstrumentedHTML(view) { // Serialize clone to HTML, then move block-marker index onto wrapper as data-block-index // (same pattern as da-nx qe-advanced: getInstrumentedHTML in prose2aem.js). - let htmlString = prose2aem(editorClone, true, false); + let htmlString = prose2aem(editorClone, true, false, true); htmlString = htmlString.replace( /
          <\/div>\s*]*?)>/gi, (_match, proseIndex, divAttributes) => ``, @@ -303,9 +303,11 @@ function getDefaultContentKind(el) { if (tag === 'UL') return { kind: 'list', ordered: false }; if (tag === 'PRE') return { kind: 'code' }; if (tag === 'BLOCKQUOTE') return { kind: 'quote' }; - // a text-less

          wraps only an image, as does a bare /; - // anything with text is a paragraph - return { kind: el.textContent?.trim() ? 'paragraph' : 'image' }; + if (el.textContent?.trim()) return { kind: 'paragraph' }; + // A text-less

          wraps only an image, as does a bare / — but a text-less + //

          with no image at all is just an empty paragraph, not an image wrapper. + if (el.matches?.('img') || el.querySelector?.('img')) return { kind: 'image' }; + return { kind: 'paragraph' }; } export function parseSections(htmlText) { From e5342435f9780e4a4367a8da4a60657fe10ade53 Mon Sep 17 00:00:00 2001 From: Sean Steimer Date: Wed, 29 Jul 2026 16:16:47 -0700 Subject: [PATCH 21/21] fix(canvas): suppress redundant null broadcast on content selection _scrollDocToProseIndex's dispatch synchronously triggers the tracking plugin's onSelectionChange, which broadcasts a null node payload (only image/table are recognized by selectedNodePayload) an instant before the correct content payload overwrites it, briefly flashing the layout-view highlight to nothing. Co-Authored-By: Claude Sonnet 5 --- blocks/canvas/ew-editor-doc/ew-editor-doc.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/blocks/canvas/ew-editor-doc/ew-editor-doc.js b/blocks/canvas/ew-editor-doc/ew-editor-doc.js index 8cefc0249..f567ed6b3 100644 --- a/blocks/canvas/ew-editor-doc/ew-editor-doc.js +++ b/blocks/canvas/ew-editor-doc/ew-editor-doc.js @@ -140,9 +140,14 @@ export class EwEditorDoc extends LitElement { const { doc } = view.state; if (proseIndex > doc.content.size) return; + // The dispatch below runs the tracking plugin's onSelectionChange synchronously, which + // would otherwise broadcast its own (null, for non-image/table selections) node payload + // an instant before the correct one just below overwrites it. + this._suppressAutoBroadcast = true; if (kind === 'image' && doc.nodeAt(proseIndex)?.type.name === 'image') { const sel = NodeSelection.create(doc, proseIndex); view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); + this._suppressAutoBroadcast = false; this._broadcastSelectedNode(true); return; } @@ -154,18 +159,21 @@ export class EwEditorDoc extends LitElement { if (nodeStart >= 0 && nodeNames?.includes(doc.nodeAt(nodeStart)?.type.name)) { const sel = NodeSelection.create(doc, nodeStart); view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); + this._suppressAutoBroadcast = false; this._broadcastSelectedNode(true, { anchorType: 'content', proseIndex }); return; } const sel = TextSelection.near(doc.resolve(proseIndex)); view.dispatch(view.state.tr.setSelection(sel).scrollIntoView()); + this._suppressAutoBroadcast = false; this._broadcastSelectedNode(true, { anchorType: 'content', proseIndex }); } // overrideNode lets content navigation (a TextSelection selectedNodePayload can't // classify) broadcast an explicit anchorType/proseIndex instead of a derived one. _broadcastSelectedNode(scrollIntoView = false, overrideNode = undefined) { + if (this._suppressAutoBroadcast && overrideNode === undefined) return; const port = this._controllerCtx?.port; const { view } = this._proseContext ?? {}; if (!port || !view) return;