Repository navigation
feat(builder): name and lock a block from the inspector - #1018
Merged
Merged
Conversation
The layers panel displays a block's name and its lock, and nothing in the editor could set either — the panel showed information the editor had no way to produce. This is the writer for both. They live in the inspector rather than in the layers row. A control inside a `role="treeitem"` complicates the roving-tabindex model `TreeView` owns, and the inspector already answers "change the selected block"; the panel reflects the result because it reads the same document. Clearing a name and releasing a lock UNSET the field rather than storing `""` or `false`. Both fields are optional, so absent is already what "no name" and "not locked" mean everywhere else — storing the falsy value would make two spellings of one state, and `layerLabel` would have to know about both to avoid rendering a blank row. Releasing a lock by writing `false` would also add a field to every block an author ever touched, meaning what its absence meant. A name is trimmed, because a name of spaces satisfies a non-empty check, renders as nothing, and cannot be reached by the layers panel's typeahead. The announcements now use that name too. They were the one surface still calling a block by its type while the panel and the breadcrumb used the name the author gave it — the same drift `blockLabel` was extracted to remove, reappearing one layer up. `deletionAnnouncement` takes a resolved name rather than a type so the rule stays in one place instead of gaining a second resolution inside the phrasing. `inspector-panel.tsx` had no test file at all; this adds one, scoped to the identity fields and saying so, because a file that exists reads as a file that covers the module.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Collaborator
Author
|
@codex please review this PR |
|
Warning Review limit reached
Next review available in: 28 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 (1)
📒 Files selected for processing (7)
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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Greptile SummaryThe PR adds inspector controls for naming and locking selected blocks, with trimmed/unset operation semantics and synchronized accessible labels.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The new identity controls produce normalized editor operations, remain synchronized with document state, and the announcement changes resolve reachable nodes through the shared display-label logic.
|
| Filename | Overview |
|---|---|
| packages/builder/src/inspector-panel.tsx | Adds controlled name and lock fields with selection-scoped state and editor operation wiring; no actionable defect identified. |
| packages/builder/src/inspector.ts | Exposes block identity during inspection and constructs trimmed rename and lock/unlock operations using unset semantics. |
| packages/builder/src/keyboard-actions.tsx | Resolves instance-aware labels for deletion and lock announcements while preserving existing keyboard behavior. |
| packages/builder/src/inspector-panel.test.tsx | Covers stored identity rendering, commit timing, no-op avoidance, unset behavior, and no-selection rendering. |
| packages/builder/src/styles/builder-chrome.css | Visually separates identity controls from editable block properties. |
Reviews (1): Last reviewed commit: "feat(builder): name and lock a block fro..." | Re-trigger Greptile
Contributor
Fallow audit reportFound 40 findings. Details
Generated by fallow. |
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.
The layers panel displays a block's name and its lock. Nothing in the editor could set either — so the panel showed information the editor had no way to produce. This is the writer for both, and it completes the loop #1013 and #1016 started.
Where they live, and why not the layers row
Plan 04's B-13 says "explicit-gesture rename; hover eye+lock", which reads as controls in the row. I put them in the inspector instead:
role="treeitem"complicates the roving-tabindex modelTreeViewowns, and that component's accessibility is the half most likely to rot if fought.Clearing UNSETS rather than storing a falsy value
Both fields are optional, so absent is already what "no name" and "not locked" mean everywhere else. Storing
""orfalsewould create two spellings of one state, andlayerLabelwould have to know about both to avoid rendering a blank row.Releasing a lock by writing
falsehas a second cost:lockedis absent on every node in every document written so far, so an unlock would become a write to every block an author ever touches, adding a field that means what its absence already meant.A name is trimmed — a name of spaces passes a non-empty check, renders as nothing, and cannot be reached by the panel's typeahead.
A drift I reintroduced this morning and have now removed
The announcements were still calling a block by its type while the panel and breadcrumb used the name its author gave it:
That is exactly the drift
blockLabelwas extracted to remove, reappearing one layer up — and the announcement is the one surface a screen-reader user hears.deletionAnnouncementnow takes a resolved name rather than a type, so the rule stays inlayerLabelinstead of gaining a second resolution inside the phrasing.Verified end to end
Tests
636 in
@nextlyhq/builder(was 620), 61 inplugin-page-builder, 30/30 tasks. Four stub-verifications on the unset and trim rules, each failing exactly its intended tests.inspector-panel.tsxhad no test file at all. This adds one, scoped to the identity fields and saying so in its header — the prop controls below remain uncovered, and a file that exists otherwise reads as a file that covers the module.One thing worth flagging from writing it: my first version of the "shows the stored lock" case contained
expect(...).toBeChecked;— the property form, which asserts nothing. It threw because this package does not register jest-dom, which is the only reason I noticed. A no-op assertion that happened to be valid syntax would have shipped as coverage.