Add up down arrows to section and children

[sid597]

Summary by CodeRabbit

Release Notes

• New Features
  • Added page reordering with move up/down controls
  • Added section reordering with move up/down controls
  • Added child reordering within sections with move up/down buttons
  • Move controls intelligently disable at boundary positions (first/last)

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[linear]

ENG-1061 Add up down (🔼🔽) arrows to each page/section in the settings panel so that they can be reordered.

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[sid597]

• Add up down arrows to section and children #560 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[sid597]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added page and section reordering functionality to left sidebar settings components. Pages can be reordered with up/down buttons, and sections along with their child items can be reordered independently. Changes persist to Roam via roamAlphaAPI.moveBlock.

Changes

Cohort / File(s)	Summary
Page reordering controls apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx	Extended PageItem props with index, isFirst, isLast, and onMove callback. Added ButtonGroup with up/down/trash actions to each page item. Implemented movePage function to reorder pages locally and persist via roamAlphaAPI.moveBlock.
Section and child reordering controls apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx	Extended SectionItem props to include index, isFirst, isLast, and onMoveSection. Added ButtonGroup controls for section reordering and per-child movement. Implemented moveSection and moveChild functions to reorder items locally and persist via roamAlphaAPI.moveBlock with refresh/notify.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as Component UI
    participant Handler as Move Handler
    participant State as Local State
    participant API as roamAlphaAPI
    
    User->>UI: Click up/down button on page/section
    activate UI
    UI->>Handler: onMove(index, direction)
    activate Handler
    Handler->>State: Reorder items in state
    State-->>Handler: State updated
    alt Has parent block (childrenUid)
        Handler->>API: moveBlock(uid, parentUid, index)
        activate API
        API-->>Handler: Block moved
        deactivate API
        Handler->>Handler: Trigger refresh/notify
    end
    deactivate Handler
    Handler-->>UI: Complete
    deactivate UI
    UI->>User: Render updated order

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

• New prop signatures on component interfaces (index, isFirst, isLast, onMove/onMoveSection) require verification that all call sites are updated correctly
• API integration with roamAlphaAPI.moveBlock needs careful review to ensure block UIDs and parent references are handled correctly
• Disabled state logic for up/down buttons must be validated against isFirst/isLast conditions to prevent invalid operations
• Dual reordering flows in LeftSidebarPersonalSettings (both section-level and child-level movements) have similar but distinct implementations that should be checked for consistency

Possibly related PRs

• ENG-853 left sidebar global and personal settings #441: Introduces the PageItem and SectionItem component foundations with index/isFirst/isLast props that this PR extends with movement controls and reordering handlers.
• ENG-882 Let user rearrange personal section and section items in place in left sidebar #456: Implements reordering for the same left-sidebar components using roamAlphaAPI.moveBlock in parallel or alternative approach.

Pre-merge checks

✅ 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 'Add up down arrows to section and children' directly reflects the main changes: adding movement controls (up/down buttons) to both sections and child items in the settings panel.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx (1)

20-34: PageItem movement wiring looks good; onRemove callback can be cleaner

• The new index/isFirst/isLast/onMove props and the ButtonGroup (up/down/trash) are wired correctly, with appropriate disabled states and clear titles.
• Minor type/clarity nit: onRemove is typed as (page: RoamBasicNode) => void, but the parent passes a zero-arg wrapper onRemove={() => void removePage(page)} and PageItem then calls onRemove(page). For consistency and to avoid subtle TS friction, consider either:
  • Changing the prop to onRemove: () => void and calling onClick={onRemove} (or () => onRemove()), or
  • Leaving the prop type as-is but passing onRemove={removePage} and calling onClick={() => void onRemove(page)}.
• This keeps the API shape obvious and avoids mixing closure-based and argument-based styles for the same callback.

Based on learnings

Also applies to: 37-60, 329-337

apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx (1)

5-11: Section movement controls are well‑wired; minor Button sizing nit

• The new SectionItem props (index, isFirst, isLast, onMoveSection) and the ButtonGroup hooks (onMoveSection(index, "up" | "down") with correct disabled states) look solid, and the parent mapping passes the right values.
• Small UI consistency point: in the section header ButtonGroup, the up/down buttons are small, but the “Add children / Edit section settings” and “Remove section” buttons are not, so they’ll render larger than the arrows and larger than the analogous controls in global settings. Consider adding small to those two buttons for a consistent look.

Also applies to: 29-48, 307-341, 593-605

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18e7685 and 01ca8a8.

📒 Files selected for processing (2)

• apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx (4 hunks)
• apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx (7 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-11T19:09:58.252Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 337
File: apps/roam/src/components/DiscourseFloatingMenu.tsx:43-43
Timestamp: 2025-08-11T19:09:58.252Z
Learning: The roam subdirectory (apps/roam) is constrained to React 17 and cannot use React 18+ features like createRoot API. ReactDOM.render should be used instead of createRoot in this subdirectory.

Applied to files:

• apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx
• apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx

🔇 Additional comments (3)

apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx (1)

142-169: Verify movePage Roam order calculation (possible off‑by‑one when moving down)

• The local reordering logic (guards for first/last, splice/remove/insert, setPages(newPages)) looks correct.
• The persistence step uses order = direction === "down" ? newIndex + 1 : newIndex when calling window.roamAlphaAPI.moveBlock. Depending on Roam’s exact order semantics (0‑based final index vs. insertion position), the + 1 for "down" could cause the page to jump more than one position.
• Since the same pattern is reused for sections/children in LeftSidebarPersonalSettings.tsx, it’d be good to confirm in Roam (e.g., with 3–4 children) that a single “move down” click moves the item by exactly one slot and adjust the formula if needed.

apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx (2)

220-263: Child reordering logic looks correct; confirm Roam order offset for down moves

• moveChild’s guards for first/last, cloning of section.children, splice‑remove‑insert, and setSections update all look correct and match the rendered disable states on the child up/down buttons.
• As with movePage, the persistence step uses order = direction === "down" ? newIndex + 1 : newIndex in window.roamAlphaAPI.moveBlock. If Roam interprets order as the final zero‑based index, the + 1 could move the child two positions when going down.
• Please verify behavior in Roam (e.g., with several children) and align this calculation with whatever you confirm is correct, keeping it consistent with the global settings reordering.

Also applies to: 375-409

---

519-546: Section reordering mirrors global behavior; double‑check moveBlock order semantics

• The moveSection handler correctly prevents moving the first/last sections out of bounds, rebuilds the sections array via splice/remove/insert, and updates state; the per‑section isFirst/isLast props and header buttons line up with that logic.
• As in movePage and moveChild, the Roam persistence uses order = direction === "down" ? newIndex + 1 : newIndex. To ensure sections move exactly one slot per click, please confirm that this formula matches Roam’s moveBlock contract and adjust (and keep in sync with global/child reordering) if needed.

Also applies to: 593-605

[mdroidian]

👍
Might be worth only showing on hover, with a slight delay (will reduce some noise).

[sid597]

https://www.loom.com/share/32d5c37800604666b0a1853878fe134d

cc @mdroidian