Skip to content

fix(canvas): allow dropping blocks into empty outline sections - #1166

Closed
shsteimer wants to merge 1 commit into
mainfrom
dropempt
Closed

fix(canvas): allow dropping blocks into empty outline sections#1166
shsteimer wants to merge 1 commit into
mainfrom
dropempt

Conversation

@shsteimer

@shsteimer shsteimer commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The outline panel's _onSectionDragOver bailed out entirely (if (!sec.blocks.length) return;) when dragging a block over a section with zero blocks, making it impossible to drop a block into an empty section.
  • Added moveBlockToSection(view, fromIndex, sectionIndex) in blocks/canvas/editor-utils/blocks.js, following the same delete-then-insert-with-offset-adjustment pattern as moveBlock, reusing getBlockPositions/getSectionStartOffset, and validating both fromIndex and sectionIndex are in range.
  • Updated _onSectionDragOver to show a drop indicator on the "No blocks" placeholder <li> (tagged data-empty-section) when dragging a block over an empty section, and updated _onDrop to dispatch moveBlockToSection for this case (disambiguated via a new emptySection flag on _dropTarget, since a sectionIndex-only drop target previously always meant "reorder whole section").
  • No CSS changes needed — the existing [data-drop-position="before"/"after"] selectors are unscoped attribute selectors, so they already apply to the empty-section placeholder <li>.

Test plan

  • npm run lint passes
  • npm test passes (1689 passed, 0 failed, 4 pre-existing skipped)
  • Added test/unit/blocks/canvas/editor-utils/blocks.test.js covering moveBlockToSection: moving a block into an empty section, out-of-range fromIndex, out-of-range sectionIndex
  • Full browser/manual verification — NOT performed. This is a collaborative, IMS-authenticated editor requiring a real org/site; no dev server or browser session was run against it in this sandbox. Please verify drag-and-drop behavior manually before merging.

🤖 Generated with Claude Code

@shsteimer @claude
fix(canvas): allow dropping blocks into empty outline sections
98a92ad 
Dragging a block over a section with zero blocks previously bailed
out entirely, making it impossible to drop a block into an empty
section from the outline panel. Add moveBlockToSection to relocate
the block to the start of the target section, wire up a drop
indicator on the empty-section placeholder, and dispatch the new
transaction from _onDrop.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@aem-code-sync

aem-code-sync Bot commented Jul 22, 2026

Copy link
Copy Markdown

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@shsteimer shsteimer mentioned this pull request Jul 23, 2026
Merged
3 tasks
@shsteimer

shsteimer commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

superseded by #1167

since I was touching a bunch of drag/drop/re-order code there, just aded this fix in

@shsteimer shsteimer closed this Jul 24, 2026
@shsteimer
shsteimer deleted the dropempt branch July 24, 2026 17:14
shsteimer added a commit that referenced this pull request Jul 24, 2026
@shsteimer @claude
fix(canvas): correct default-content proseIndex resolution; let block…
605da1e 
…s/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 <noreply@anthropic.com>
shsteimer added a commit that referenced this pull request Jul 30, 2026
@shsteimer @claude @hannessolo
feat(canvas): surface default content in outline panel (#1167)
057e837 
* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* fix(canvas): rerender on block identity changes; add code blocks to outline

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 <pre> 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 <noreply@anthropic.com>

* fix(canvas): address outline default-content review feedback

- getDefaultContentProseIndex: for kind 'image', prefer the nested
  data-image-index over a wrapping <p>'s own data-prose-index so the
  image node resolves correctly in the doc
- getDefaultContentKind: classify <blockquote> as 'quote' and make the
  fallback content-aware so text-bearing tags aren't mislabeled 'image';
  drop the now-redundant <p> 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) <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* feat(canvas): drag-reorder and delete for default-content items

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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* fix(canvas): highlight the selected default-content item in the outline

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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* feat(canvas): sync default-content selection to layout view and outline

Selecting a default-content item (paragraph/heading/list/code/quote) from
the outline now scrolls and highlights it in both the content and layout
views, matching existing block/image behavior. Also fixes the reverse
direction: editing or selecting content directly in the content view (or
via layout, once synced) now highlights the matching row in the outline,
which previously only tracked block selections.

Adds blockquote to EDITABLES so quote items get their own data-prose-index
instead of falling back to a nested paragraph's.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(canvas): use blue text for selected content items in outline

.content-label and .content-snippet had their own explicit gray color,
which overrode the blue text .block-item.selected already applies —
blocks don't hit this since .block-name has no such override.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(canvas): show first-line snippet for outline content children

Previously showed the full concatenated text of a content child, which
overflowed for long paragraphs/lists. Now derives a per-kind first-line
snippet (first list item, first quote paragraph, text before any <br>)
and relies on existing CSS text-overflow: ellipsis for visual truncation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(canvas): selection-driven outline expansion, empty-node visibility, move/delete selection

Replaces the fragile proseIndex-persistence model for default-content expand/collapse
with one that resets on every selection change: block selection collapses all runs,
content selection expands only its own run, and manual expand/collapse persists only
until the next selection change. This shrinks proseIndex drift exposure to within a
single selection instead of indefinitely across arbitrary doc edits.

Also:
- Empty default-content nodes (e.g. an empty heading/paragraph) now stay in the parsed
  tree flagged `empty`, hidden in the outline unless currently selected — so a node
  being actively edited is visible/selectable. Required keeping empty <p> 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* fix(canvas): keep prose2aem free of canvas-only empty-paragraph handling

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 <p>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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* fix(canvas): restore image detection in getDefaultContentKind, drop dead prose2aem arg

getDefaultContentKind now explicitly checks for a nested/matching <img> 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Hannes Hertach <hhertach@adobe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shsteimer @maximilianvoss