Repository navigation
feat(builder): layers panel and ancestor breadcrumb - #1013
Merged
Merged
Conversation
The canvas shows a page. It does not show that a heading is inside a box which is inside a section, and it cannot show a block that renders nothing — an empty container is a couple of pixels and a block hidden at the current breakpoint is not there at all. Those are the blocks an author loses. `layers.ts` derives the tree, the path to a block and what a search leaves standing. The panel and the breadcrumb both read it, so the trail and the panel's highlight cannot name different ancestors. The tree is `@nextlyhq/ui`'s `TreeView` rather than one written here: it is virtualized, and its flat `role="treeitem"` rows carry the level and position attributes that a virtualized tree needs because nested `role="group"` markup cannot be built when only a window is rendered. Reimplementing that would put the accessibility half where it would rot. Children are flattened across slots. Measured across the catalogue: nine containers declare exactly one slot and none declares more, so a flat tree describes the document faithfully and slot rows would add a level of nesting to name a distinction no block currently makes. Badges are three separate facts rather than one eye. `locked` is a boolean the engine defines; visibility is conditions evaluated against an entry the editor cannot see, and a per-breakpoint map where a block hidden on mobile is fully present on desktop. One icon would have to state an answer nothing here knows. Expansion is split by how long it should last. A selection opens its ancestors once, into the author's own set, and the author may close them again; a search opens its matches' ancestors only while the query stands. Deriving all three as one union reads correctly and makes an ancestor of the selected block impossible to close. One label rule, from `blockLabel`. The inspector's `editor.label ?? node.type` was a second rule that agreed only for blocks declaring a label, so an unlabelled third-party block was "Collection loop" in the palette and `acme/collection-loop` in the inspector.
Clicking a block on the canvas cleared the selection instead of setting it. `setPointerCapture` on `pointerdown` retargets every later pointer event to the capturing element, and a browser derives a `click`'s target from where the press and the release landed. Capturing on the press therefore made every click on the canvas report the canvas ROOT as its target; the hit test walks up from there, finds no block above the root, and reads that as a click on the background. Capture waits for activation instead. A drag needs it because the pointer may leave the canvas and must keep delivering moves; a click does not, and until the pointer has travelled far enough there is no way to tell which one a press will turn out to be. jsdom implements no capture retargeting and synthesises no click from a press, so the symptom is not reproducible in a unit test — the test asserts WHEN capture is taken, which is what the defect was, with the activation case as its positive control.
The trail itself is asserted in `layers.test.ts` without a DOM. What is only true in the component is which crumb is marked current, that every crumb changes the selection, and that an empty path renders nothing rather than an empty bar — which is the case an undo reaches routinely, by removing the selected node while the selection stands.
Collaborator
Author
|
@codex please review this PR |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 56 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (16)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryAdds a virtualized layers tree, searchable structural model, ancestor breadcrumb, and page-builder shell integration, while also changing canvas pointer capture to restore click selection.
Confidence Score: 2/5The PR is not yet safe to merge because pointer gestures can remain stale and the layers panel can hide selected or searched-for descendants. Delayed pointer capture lacks cleanup for releases outside the canvas, selection expansion is not recomputed after moves, and matching collapsed containers are not temporarily expanded during search; the duplicate changesets also need consolidation. Files Needing Attention: packages/builder/src/canvas-drag.tsx, packages/builder/src/layers-panel.tsx, packages/builder/src/layers.ts, and the two new changeset files
|
| Filename | Overview |
|---|---|
| packages/builder/src/canvas-drag.tsx | Delays pointer capture until activation but leaves an unactivated gesture stale when release occurs outside the canvas. |
| packages/builder/src/layers-panel.tsx | Implements tree rendering and temporary expansion, but does not refresh selection ancestors when the selected node moves. |
| packages/builder/src/layers.ts | Adds hierarchy and search derivation, but direct container matches are not temporarily expanded to reveal their retained children. |
| packages/builder/src/breadcrumb.tsx | Renders the selected node's ancestor path and selects crumbs without an identified defect. |
| packages/plugin-page-builder/src/admin/BlocksField.tsx | Integrates the new layers panel and breadcrumb into the full-screen builder shell. |
| .changeset/canvas-click-selects-again.md | Introduces one of two all-package changesets where repository policy requires a single combined changeset. |
| .changeset/layers-panel-and-breadcrumb.md | Introduces the second all-package changeset that should be consolidated with the first. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Document[BlockDocument] --> Layers[layersOf / filterLayers]
Selection[Editor selection] --> Expansion[Ancestor expansion]
Layers --> Panel[LayersPanel / TreeView]
Expansion --> Panel
Document --> Path[pathTo]
Selection --> Path
Path --> Breadcrumb[SelectionBreadcrumb]
Panel -->|select row| Selection
Breadcrumb -->|select ancestor| Selection
Prompt To Fix All With AI
### Issue 1
packages/builder/src/canvas-drag.tsx:237
**Unreleased presses leave stale gestures**
When a press leaves the canvas before crossing the activation threshold and is released outside, the root receives no release or cancellation because capture has not started. The stored gesture then activates on a later hover without checking the pointer or button state, allowing the old block to be selected or moved without a current drag.
### Issue 2
packages/builder/src/layers-panel.tsx:140
**Moved selections lose ancestor expansion**
When the selected block is dragged or keyboard-indented into a collapsed container, the document changes while `selectedId` remains the same, so this effect does not add the new ancestors to `opened`. `TreeView` then omits the selected descendant, causing the selected block to disappear from the layers panel.
### Issue 3
packages/builder/src/layers.ts:206-210
**Matching containers remain collapsed**
When a query directly matches a collapsed container, this branch retains its children but does not add the container to `expand`. Because the panel expands only author-opened IDs and `search.expand`, the search displays the container while hiding the subtree it is documented to reveal.
### Issue 4
.changeset/canvas-click-selects-again.md:1
**Duplicate lockstep changesets**
This PR adds two separate all-packages patch changesets, while the repository's lockstep release policy requires one changeset per PR. Consolidating them preserves a single release entry for the combined stacked changes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "test(builder): cover the breadcrumb's ow..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // delivering. Without it a drag that wanders outside simply stops | ||
| // reporting, and the gesture ends wherever the pointer happened to exit. | ||
| root.setPointerCapture(event.pointerId); | ||
| // NOT captured here. See the activation branch in `onPointerMove`. |
There was a problem hiding this comment.
When a press leaves the canvas before crossing the activation threshold and is released outside, the root receives no release or cancellation because capture has not started. The stored gesture then activates on a later hover without checking the pointer or button state, allowing the old block to be selected or moved without a current drag.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/builder/src/canvas-drag.tsx
Line: 237
Comment:
**Unreleased presses leave stale gestures**
When a press leaves the canvas before crossing the activation threshold and is released outside, the root receives no release or cancellation because capture has not started. The stored gesture then activates on a later hover without checking the pointer or button state, allowing the old block to be selected or moved without a current drag.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| // effect re-rendering forever: a new array every run is a new state value. | ||
| return missing.length === 0 ? previous : [...previous, ...missing]; | ||
| }); | ||
| }, [editor.selectedId]); |
There was a problem hiding this comment.
When the selected block is dragged or keyboard-indented into a collapsed container, the document changes while selectedId remains the same, so this effect does not add the new ancestors to opened. TreeView then omits the selected descendant, causing the selected block to disappear from the layers panel.
Knowledge Base Used: Page Builder plugin
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/builder/src/layers-panel.tsx
Line: 140
Comment:
**Moved selections lose ancestor expansion**
When the selected block is dragged or keyboard-indented into a collapsed container, the document changes while `selectedId` remains the same, so this effect does not add the new ancestors to `opened`. `TreeView` then omits the selected descendant, causing the selected block to disappear from the layers panel.
**Knowledge Base Used:** [Page Builder plugin](https://app.greptile.com/nextly/-/custom-context/knowledge-base/nextlyhq/nextly/-/docs/plugin-page-builder.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+206
to
+210
| if (matches(node, needle)) { | ||
| // Kept whole. Its descendants are the contents of a thing the author | ||
| // asked for, and hiding them would answer a narrower question than the | ||
| // one they typed. | ||
| return [node]; |
There was a problem hiding this comment.
When a query directly matches a collapsed container, this branch retains its children but does not add the container to expand. Because the panel expands only author-opened IDs and search.expand, the search displays the container while hiding the subtree it is documented to reveal.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/builder/src/layers.ts
Line: 206-210
Comment:
**Matching containers remain collapsed**
When a query directly matches a collapsed container, this branch retains its children but does not add the container to `expand`. Because the panel expands only author-opened IDs and `search.expand`, the search displays the container while hiding the subtree it is documented to reveal.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| @@ -0,0 +1,32 @@ | |||
| --- | |||
There was a problem hiding this comment.
This PR adds two separate all-packages patch changesets, while the repository's lockstep release policy requires one changeset per PR. Consolidating them preserves a single release entry for the combined stacked changes.
Rule Used: In the nextlyhq/nextly repo, every PR must include... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: .changeset/canvas-click-selects-again.md
Line: 1
Comment:
**Duplicate lockstep changesets**
This PR adds two separate all-packages patch changesets, while the repository's lockstep release policy requires one changeset per PR. Consolidating them preserves a single release entry for the combined stacked changes.
**Rule Used:** In the nextlyhq/nextly repo, every PR must include... ([source](https://app.greptile.com/nextly/github/nextlyhq/nextly/-/custom-context?memory=376ea7a2-1076-49e6-bbd9-dd342f286b4e))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
@nextlyhq/eslint-plugin
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
Contributor
Fallow audit reportFound 41 findings. Dependencies (40)
Health (1)
Generated by fallow. |
github-actions
Bot
added
type: docs
This was referenced Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan 04 B-13 (layers panel) and the navigation half of B-16 (ancestor breadcrumb).
Why this, now
The editor can insert, select, move, delete, undo and edit. It could not show an author the structure of their page — and structure is exactly where blocks get lost: an empty container is a couple of pixels, and a block hidden at the current breakpoint is not on the canvas at all.
The tree is
@nextlyhq/ui'sTreeView, not one written hereF7 shipped it and its own docblock says it exists for this control. It is virtualized, and its flat
role="treeitem"rows carryaria-level/aria-setsize/aria-posinsetbecause virtualization makes the nestedrole="group"markup impossible to build — only a window is in the DOM. It also brings the APG keyboard model, roving tabindex and typeahead. Reimplementing that here would have put the accessibility half in the place it would rot.Decisions, each from a measurement
Children are flattened across slots. Measured across the catalogue: 9 containers declare exactly one slot, none declares more. So a flat tree is faithful today, and slot rows would add a level of nesting to name a distinction no block currently makes. The limit is stated in the module — a two-slot block would show its children in one run, and this is where slot rows grow.
Badges are three facts, not one eye.
lockedis a boolean the engine defines. Visibility is not: it isconditionsevaluated against an entry the editor cannot see, plus a per-breakpoint map where a block hidden on mobile is fully present on desktop. A single eye icon would have to state an answer nothing here knows, so the row reports locked, hidden at some screen sizes and shown conditionally separately — each as clipped text beside anaria-hiddenicon, because an icon alone announces as an image with no name.Expansion is split by how long it should last. Three things want to open a branch: the author, a selection made on the canvas, a search. Deriving all three as one union reads correctly and makes an ancestor of the selected block impossible to close — the author collapses it, the next render puts it straight back. So a selection opens its ancestors once, into the author's own set; a search opens its matches' ancestors only while the query stands. Only the author's set is stored.
One label rule.
blockLabelreplaces the inspector'seditor.label ?? node.type, which agreed only for blocks that declare a label — an unlabelled third-party block was "Collection loop" in the palette andacme/collection-loopin the inspector. This also changes a delete announcement for an unlabelled block fromacme/textto "Text"; that test now records the decision and why.Verified in a browser
Tests
606 in
@nextlyhq/builder(was 575), 61 inplugin-page-builder, 30/30 tasks. Every rule stub-verified — broken one at a time, intended tests confirmed failing, restored, restore confirmed by diff: 5 for the tree model, 3 for the expansion rules, 4 for the breadcrumb.One of those breaks moved nothing, which is a finding rather than a pass: dropping the search-branch filter changed no result, because the filter only runs when the author toggles a branch and no test toggled one during a search. The property was uncovered. The test that reaches it is in this PR, and re-running the same break now fails it.
Not in this PR, deliberately
Rename, the lock toggle, and the 2.5.7 move menus. The lock toggle needs
node.lockedto be honoured by delete and keyboard-move too — today only drag reads it — and shipping a switch whose effect is half-wired is worse than shipping none. That is a coherent "locking works" PR of its own.Note for whoever reviews the chrome
packages/builder/src/**is outside the three design-token ESLint rules, outsidelint-design.mjs, and outside the alpha-contrast suite's scanned directories. Nothing would catch a sub-3:1 border in this panel. The new styles therefore use only the existing--nx-builder-*tokens, which alias the vetted admin palette, and add no opacity-modified colours — an alpha-blended foreground's contrast depends on whatever is behind it, which is what brokemainearlier today. That coverage gap is worth closing separately.