Skip to content

Drag-and-drop respects add/edit/delete permissions (relocate vs reorder)#386

Merged
CarlosNZ merged 3 commits into
mainfrom
drag-n-drop-improvements
Jun 26, 2026
Merged

Drag-and-drop respects add/edit/delete permissions (relocate vs reorder)#386
CarlosNZ merged 3 commits into
mainfrom
drag-n-drop-improvements

Conversation

@CarlosNZ

Copy link
Copy Markdown
Owner

What

Makes drag-and-drop honour the allow* permission filters consistently. A move is delete-from-source + add-to-destination, but only the source half was gated before — drops were allowed wherever the destination was editable, and allowAdd had no effect, so a node could be dragged into a collection that forbade adds. This distinguishes a reorder (drop within the same collection) from a relocate (drop into a different one) and gates each correctly.

Surfaced by the Playlist example (#384 follow-up): allowAdd={false} hid the "+" buttons but didn't stop a track being dragged out of its list.

Rules enforced

A drop inserts the dragged node as a sibling of the target, into the target's parent ("destination collection").

Action Condition
Pick up (any drag) allowDrag on the source — decoupled from delete
Reorder (same collection) allowDrag(source) + allowEdit(destination collection)
Relocate (different collection) allowDrag(source) + allowDelete(source) + allowAdd(destination collection)

Self/descendant and KEY_EXISTS guards are unchanged. Enforced in the UI/interaction layer (handleDrop + the drag-aware drop highlight), consistent with how all other edits are gated — move has a single entry point, so this is complete. No commit-engine changes.

How

The decision logic is concentrated in useDragNDrop.tsx: a dropAllowed() predicate compares source vs. destination parent and applies the rules. getDropTargetProps, BottomDropTarget, and handleDrop share it, so the highlight only lights up legal targets and the drop enforces the same verdict.

Everything else is mechanical threading mirroring the existing canDragOnto pattern:

  • useCommon: canDrag decoupled from delete (pickup = allowDrag only).
  • DragSourceProvider: DragSource carries the source's canDelete, stashed at pickup.
  • types.ts: new canAddHere on BaseNodeProps (additive); canDragOnto kept (now the reorder signal).
  • CollectionNode / ValueNodeWrapper / JsonEditor: thread canAddHere + canDelete through.

Compatibility (semver-significant)

  • allowDelete:false no longer makes a node undraggable — it can still be reordered in place; use allowDrag:false to disable dragging entirely.
  • A destination with allowAdd:false now rejects incoming relocations (it previously accepted them if editable).

The Playlist example now confines drags to the list for free via its existing allowAdd={false} + deletable items — comment/blurb updated to match.

Tests

test/dragAndDrop.test.tsx: updated the old allowDelete-gates-drag test to the new contract, plus 6 new tests (reorder-without-delete, reorder-blocked-when-not-editable, relocate blocked on no-delete / no-add, relocate allowed, and a Playlist-shaped regression). 678 tests pass, lint + tsc clean, demo typecheck clean, core build succeeds.

Docs

README_V2 drag-and-drop rules table, migration-guide §5 behaviour note, CHANGELOG entry under 2.0.0-beta.5.

Not done in this PR

  • package.json left at beta.4 — version bump is yours to make at publish time.
  • Firefox manual check still wanted (jsdom can't reproduce native DnD edge cases): /examples/playlist — reorder within the list works; dragging a track onto the title or into another track is refused; Add track still works.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

Bundle size impact

json-edit-react

Format Base raw PR raw Δ raw Base gzip PR gzip Δ gzip
esm 57.71 KB 57.99 KB 🔺 +287 B (+0.49%) 20.61 KB 20.70 KB 🔺 +91 B (+0.43%)
cjs 59.21 KB 59.49 KB 🔺 +292 B (+0.48%) 20.64 KB 20.74 KB 🔺 +98 B (+0.46%)

Measured from build/index.{cjs,esm}.js. Gzip at level 9.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the drag-and-drop interaction layer to consistently enforce allow* permission filters by distinguishing reorder (within the same collection) from relocate (across collections), and threads the required permission signals through the node component tree. It also updates tests and documentation to reflect the new contract (notably: pickup is controlled by allowDrag only, while relocate additionally depends on allowDelete + allowAdd).

Changes:

  • Introduces a shared dropAllowed() predicate in useDragNDrop and extends drag-source state to carry canDelete from pickup to drop.
  • Decouples “draggable/pickup” from delete permission (canDrag = allowDragFilter(nodeData)), and adds canAddHere to represent destination-collection add permission for relocate.
  • Expands/updates test coverage and updates README, migration guide, demo blurbs, and changelog to document the new rules.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/sliceIsolation.test.tsx Updates drag-source slice typing to include canDelete.
test/dragAndDrop.test.tsx Reworks existing drag permission test and adds relocate-vs-reorder coverage.
src/ValueNodeWrapper.tsx Threads canAddHere/canDelete into useDragNDrop.
src/types.ts Adds canAddHere to node props and documents reorder vs relocate semantics.
src/JsonEditor.tsx Provides canAddHere at the root node props boundary.
src/hooks/useDragNDrop.tsx Centralizes drop legality into dropAllowed() and aligns highlight/drop enforcement.
src/hooks/useCommon.ts Changes canDrag to depend only on allowDrag, not delete permission.
src/hooks/DragSourceProvider.tsx Extends drag-source context state to include canDelete.
src/CollectionNode.tsx Threads canAddHere and passes destination add permission down to children.
README_V2.md Updates drag-and-drop docs with reorder/relocate rules table.
migration-guide.md Adds a migration note explaining the changed drag-and-drop permission model.
demo/src/examples/static/playlist/Example.tsx Updates playlist example commentary to match new relocate gating.
demo/src/examples/registry.ts Updates playlist blurb to describe the new drag confinement behavior.
CHANGELOG.md Adds a 2.0.0-beta.5 entry describing the behavioral change.
Comments suppressed due to low confidence (1)

src/hooks/useDragNDrop.tsx:117

  • onDragOver always calls preventDefault() for any node that could ever accept a reorder/relocate, even when the current in-flight drag is not allowed by dropAllowed() (e.g., source can’t delete, or relocating into a collection that rejects adds). This can make the browser indicate the target is droppable and still trigger drop events, even though handleDrop will no-op. Consider only preventing default when dropAllowed() is true so illegal targets behave consistently with the highlight logic.
        onDragOver: (e: React.DragEvent) => {
          e.stopPropagation()
          e.preventDefault()
        },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CarlosNZ CarlosNZ added the V2 To include in Version 2 label Jun 26, 2026
@CarlosNZ CarlosNZ added this to the v2.0 milestone Jun 26, 2026
@CarlosNZ CarlosNZ merged commit d8c83a7 into main Jun 26, 2026
2 checks passed
@CarlosNZ CarlosNZ deleted the drag-n-drop-improvements branch June 26, 2026 22:04
CarlosNZ added a commit that referenced this pull request Jun 27, 2026
)

A key-rename is a delete of the old key + an add of the new one to the
PARENT collection, so gate it as exactly that: the node must be
deletable AND its parent collection must accept adds. The add-half now
reads the parent's allowAdd (via the `canAddHere` prop #386 already
threads down) instead of the node's own, and allowEdit no longer takes
part — a value-locked key can still be renamed.

This fixes the footgun where restricting allowAdd to a few collections
silently locked every key in the tree, and makes "can a key appear in
collection X?" a single answer (allowAdd(X)) across the add button,
rename, and drag relocate.

- src/hooks/useCommon.ts: rewrite the canEditKey gate.
- test/JsonEditor.test.tsx: 3 regression tests (red before, green
  after) + an array-index guard.
- demo edit-restrictions example: redesigned as self-documenting matrix
  data (addable / sealed / editable boxes), mirroring drag-drop-rules;
  the `editable` box shows raw-JSON editing overriding per-leaf rules.
- README_V2 NOTE, migration-guide §5 subsection, CHANGELOG beta.6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CarlosNZ added a commit that referenced this pull request Jun 27, 2026
…374) (#387)

* Key-rename permission checks the parent's allowAdd, not the node's (#374)

A key-rename is a delete of the old key + an add of the new one to the
PARENT collection, so gate it as exactly that: the node must be
deletable AND its parent collection must accept adds. The add-half now
reads the parent's allowAdd (via the `canAddHere` prop #386 already
threads down) instead of the node's own, and allowEdit no longer takes
part — a value-locked key can still be renamed.

This fixes the footgun where restricting allowAdd to a few collections
silently locked every key in the tree, and makes "can a key appear in
collection X?" a single answer (allowAdd(X)) across the add button,
rename, and drag relocate.

- src/hooks/useCommon.ts: rewrite the canEditKey gate.
- test/JsonEditor.test.tsx: 3 regression tests (red before, green
  after) + an array-index guard.
- demo edit-restrictions example: redesigned as self-documenting matrix
  data (addable / sealed / editable boxes), mirroring drag-drop-rules;
  the `editable` box shows raw-JSON editing overriding per-leaf rules.
- README_V2 NOTE, migration-guide §5 subsection, CHANGELOG beta.6.

* Refine edit-restrictions blurb: three boxes, scope to non-DnD

Reads from the README "Controlling editing" / permissions context: lead
with the three editing permissions (not including drag-n-drop), and
introduce all three boxes up front (addable / sealed / editable) rather
than "two boxes" + a later third. Same wording fix in the example's
source comment.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

V2 To include in Version 2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants