Skip to content

feat: disable merge during rearrange#557

Merged
NagariaHussain merged 2 commits intofrappe:developfrom
Rl0007:fix/sidebar-reorder
Feb 20, 2026
Merged

feat: disable merge during rearrange#557
NagariaHussain merged 2 commits intofrappe:developfrom
Rl0007:fix/sidebar-reorder

Conversation

@Rl0007
Copy link
Contributor

@Rl0007 Rl0007 commented Feb 19, 2026

Screen.Recording.2026-02-19.at.5.17.08.PM.mov

Summary by CodeRabbit

  • New Features
    • Added drag and reorder state tracking for wiki trees with user-facing feedback.
    • Merge buttons are now automatically disabled during active reordering operations to prevent conflicts.
    • Reorder operation status is now propagated through the component hierarchy for coordinated state management.

@Rl0007
Copy link
Contributor Author

Rl0007 commented Feb 19, 2026

Closes #553

@coderabbitai
Copy link

coderabbitai bot commented Feb 19, 2026

Walkthrough

The changes implement drag and reorder state management across multiple components. A new drag-state-change event is added to NestedDraggable to propagate drag events upward. WikiDocumentList aggregates drag state into a coordinated reorder flow with a debounce mechanism and emits a new reorder-state-change event. ContributionBanner receives a new mergeDisabled prop. SpaceDetails wires these components together by listening to the reorder-state-change event and binding the reordering state to the mergeDisabled prop on ContributionBanner.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: disabling the merge feature during rearrange operations, which is the primary objective reflected across all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
frontend/src/components/WikiDocumentList.vue (1)

570-580: Consider guarding against post-unmount side effects from in-flight reorders.

If the component unmounts while reorderInFlight is true, the applyReorder promise's finally block (Lines 322–329) will still execute, calling flushReorder() and mutating refs on an unmounted component. In Vue 3 this is benign (emit becomes a no-op, ref writes are harmless), but it's a latent concern if the logic ever grows more complex.

A lightweight guard would be:

💡 Optional: add an `isMounted` guard
+let isMounted = true;
+
 onBeforeUnmount(() => {
+    isMounted = false;
     if (reorderTimer) {
         clearTimeout(reorderTimer);
         reorderTimer = null;
     }

Then in flushReorder's finally:

     } finally {
         reorderInFlight = false;
+        if (!isMounted) return;
         if (pendingReorder) {
             flushReorder();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/WikiDocumentList.vue` around lines 570 - 580, The
component can be unmounted while applyReorder is still in flight causing
flushReorder's finally to mutate refs and emit on an unmounted component; add a
simple isMounted boolean (set true in onMounted and false in onBeforeUnmount)
and check it at the start of flushReorder's finally block (and before any ref
writes or emit calls) so flushReorder no-ops if the component is unmounted,
keeping existing logic in applyReorder and onBeforeUnmount intact; reference
functions/refs: applyReorder, flushReorder, reorderInFlight, onBeforeUnmount,
isReorderBusy, isDragActive, pendingReorder and the 'reorder-state-change' emit.
frontend/src/components/NestedDraggable.vue (1)

378-384: Unconditional drag-state-change(false) on unmount is fine but slightly noisy.

Every NestedDraggable instance (including deeply nested ones) emits drag-state-change(false) on unmount even when no drag is active. Since WikiDocumentList.handleDragStateChange just assigns the value, last-write-wins to false is correct and harmless. Optionally, guard the emit:

💡 Optional: guard the unmount emission
 onBeforeUnmount(() => {
     if (dragSettleTimer) {
         clearTimeout(dragSettleTimer);
         dragSettleTimer = null;
     }
-    emit('drag-state-change', false);
+    if (isDragging.value || dragSettleTimer) {
+        emit('drag-state-change', false);
+    }
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/NestedDraggable.vue` around lines 378 - 384,
Unconditional emit of 'drag-state-change', false in
NestedDraggable.onBeforeUnmount is noisy; change the unmount handler to only
emit when this instance actually has an active drag (e.g., check a local
isDragging flag or the existing dragSettleTimer) so you only call
emit('drag-state-change', false) if a drag is in progress; update
NestedDraggable (onBeforeUnmount) to guard the emit using the local dragging
indicator (or dragSettleTimer) and leave WikiDocumentList.handleDragStateChange
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/src/components/NestedDraggable.vue`:
- Around line 378-384: Unconditional emit of 'drag-state-change', false in
NestedDraggable.onBeforeUnmount is noisy; change the unmount handler to only
emit when this instance actually has an active drag (e.g., check a local
isDragging flag or the existing dragSettleTimer) so you only call
emit('drag-state-change', false) if a drag is in progress; update
NestedDraggable (onBeforeUnmount) to guard the emit using the local dragging
indicator (or dragSettleTimer) and leave WikiDocumentList.handleDragStateChange
unchanged.

In `@frontend/src/components/WikiDocumentList.vue`:
- Around line 570-580: The component can be unmounted while applyReorder is
still in flight causing flushReorder's finally to mutate refs and emit on an
unmounted component; add a simple isMounted boolean (set true in onMounted and
false in onBeforeUnmount) and check it at the start of flushReorder's finally
block (and before any ref writes or emit calls) so flushReorder no-ops if the
component is unmounted, keeping existing logic in applyReorder and
onBeforeUnmount intact; reference functions/refs: applyReorder, flushReorder,
reorderInFlight, onBeforeUnmount, isReorderBusy, isDragActive, pendingReorder
and the 'reorder-state-change' emit.

@Rl0007
Copy link
Contributor Author

Rl0007 commented Feb 20, 2026

@NagariaHussain

@NagariaHussain NagariaHussain merged commit 46414a7 into frappe:develop Feb 20, 2026
4 checks passed
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.

2 participants