Skip to content

fix(ui): canvas scroll (#7) and cross-group drag/drop (#9) - #10

Merged
RyanMakesAndBreaksStuff merged 2 commits into
mainfrom
claude/issue-7-staged-review-i3ixye
Aug 30, 2026
Merged

fix(ui): canvas scroll (#7) and cross-group drag/drop (#9)#10
RyanMakesAndBreaksStuff merged 2 commits into
mainfrom
claude/issue-7-staged-review-i3ixye

Conversation

@RyanMakesAndBreaksStuff

@RyanMakesAndBreaksStuff RyanMakesAndBreaksStuff commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Two independent fixes, one per ticket, kept as two reviewable commits:


1 — Canvas scroll (#7)

The height half of "you lose elements entirely as the screen gets smaller". The width breakpoints and the workspace row floor already landed on main; this fixes what the floor exposed.

.eb-group-card sets flex-shrink: 0 so nested groups keep their natural height. The root card inherited it, so it grew to its full content height. .eb-group-children then sized to that expanded parent rather than the space available, its overflow: auto never engaged, and the scroll fell outward to .eb-pane-body.

Measured live in PPTB at a 420px frame with six rules:

element before (scroll/client) after
.eb-workspace 496 / 350 — scrolling 496 / 350 — scrolling (the row floor, by design)
.eb-canvas-card .eb-pane-body 1426 / 353 — scrolling 353 / 353 — not scrolling
.eb-group-children 1334 / 1334 — inert 1334 / 261 — scrolling

flex: 1 1 auto restores shrink on the root; min-height: 0 defeats the automatic minimum that would otherwise still block it. Nested cards keep flex-shrink: 0. Confirmed by hand in the host: the rule list auto-scrolls under a drag while the header and docks stay put, which is the behaviour that was missing.

2 — Cross-group drag/drop (#9 item 2)

"When drag/drop'ing it seems like you should be able to move a property inside of a created group, however it just shifts it above/below."

resolveDragDropCommand rejected any condition-node drop whose target separator belonged to a different group, so every position inside a group failed useDroppable's accept and never became a target. Only same-parent reordering was reachable. Toolbox fields were exempt, which is why dragging a new field into a group already worked and moving an existing rule in did not.

The pieces were all present and unconnected: moveNode in queryActions is implemented, guarded and tested, exported from the package index, and called by nothing outside its own tests; the separators are already rendered recursively for every group; the screen-reader announcements already name the destination group. This wires them together.

Verification

  • npm run lint, npm run typecheck, npm test — 267 tests, all green.
  • New: builderDragDropProvider.test.tsx (4 tests) covering the command→callback dispatch that was the missing link, and two CSS-contract tests in dragDropStyles.test.ts.
  • Mutation-checked both fixes. Reverting .is-root to flex-grow: 1 and dropping the preview cap fails 3 tests; making resolveDragDropCommand reject cross-group drops again fails 5.
  • The UI feedback #7 numbers come from the real PPTB host (Electron, tool loaded over pptb-webview:) with the terminal panel open, not a simulated viewport.

Known gaps

  • Keyboard parity for the move. ConditionMoveButtons still only walks a node among its siblings, so this opens a gap where the mouse can do something the keyboard cannot.
    -- UI Review Issue 2 #9 items 1, 3 and 4 (Apply Wrap discoverability, padding, the theme toggle) are untouched.

claude added 2 commits August 30, 2026 19:59
Issue #7 reported elements disappearing as the window narrows. The width
half is already fixed; this is the height half, measured inside PPTB.

.eb-group-card sets flex-shrink: 0 so nested groups keep their natural
height. The root card inherited it, which made it grow to its full content
height: .eb-group-children then sized to that expanded parent rather than
the space available, its overflow: auto never engaged, and the scroll fell
outward to .eb-pane-body. Measured in PPTB at a 420px frame with six rules,
the list rendered 1334px tall inside a 398px canvas, with .eb-group-children
reporting an inert 1334/1334 and .eb-pane-body 1426/353.

Restoring flex: 1 1 auto on the root, with min-height: 0 to defeat the
automatic minimum, moves the scroll into .eb-group-children where the
existing overflow: auto always intended it. Verified in PPTB: pane-body
353/353, group-children 1334/261, header and docks static while the list
auto-scrolls under a drag.

Also caps .eb-preview-card, which is content-sized and unbounded. The
canvas is its only flexible sibling, so a long expression came straight out
of the canvas. The cap sits above the 178px an empty document measures, so
it bounds growth rather than clipping the common case; the flex column and
min-height: 0 on the body are what make it scroll rather than clip. The cap
is released below 900px, where the stacked layout scrolls as one page and
the two cards no longer compete.

Refs #7

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jw48xEwyA9pCD59WJJMPXU
Issue #9 item 2: "when drag/drop'ing it seems like you should be able to
move a property inside of a created group, however it just shifts it
above/below."

resolveDragDropCommand rejected any condition-node drop whose target
separator belonged to a different group, so every position inside a group
failed useDroppable's accept and never became a target. Only same-parent
reordering was reachable. Toolbox fields were unaffected, which is why
dragging a new field into a group already worked and moving an existing
rule in did not.

The pieces were all present and unconnected: moveNode in queryActions is
implemented, guarded and tested, exported from the package index, and
called by nothing outside its own tests; the drop separators are rendered
recursively for every group; the screen-reader announcements already name
the destination group. This wires them together.

- resolveDragDropCommand emits a move-node command for a cross-group drop.
  Its index is used as-is: the node leaves a different list, so nothing
  shifts in the target and the reorder path's -1 correction would land it
  one position too high.
- resolveCurrentDragDropCommand validates the move against the tree, with
  an inclusive bound (a move may append past the last child, a reorder may
  not) and a guard against a group landing inside itself.
- ConditionPositionTarget takes the ancestor chain, so a group dragged onto
  its own descendants stays visibly ineligible rather than lighting up as
  valid and then being rejected on drop. That is now the only case the
  is-ineligible styling covers.
- The provider dispatches move-node to a new onMoveNode, which the shell
  services with moveNode.

Not addressed: keyboard parity. ConditionMoveButtons still only walks a
node among its siblings, so this opens a gap where the mouse can do
something the keyboard cannot. Worth a follow-up before release.

Refs #9

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jw48xEwyA9pCD59WJJMPXU
@RyanMakesAndBreaksStuff RyanMakesAndBreaksStuff changed the title fix(ui): let the rule list scroll instead of overflowing the canvas fix(ui): canvas scroll (#7) and cross-group drag/drop (#9) Aug 30, 2026
@RyanMakesAndBreaksStuff RyanMakesAndBreaksStuff linked an issue Aug 30, 2026 that may be closed by this pull request
@RyanMakesAndBreaksStuff
RyanMakesAndBreaksStuff merged commit 9dc53fb into main Aug 30, 2026
4 checks passed
@RyanMakesAndBreaksStuff
RyanMakesAndBreaksStuff deleted the claude/issue-7-staged-review-i3ixye branch September 2, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI feedback

2 participants