Skip to content

Conversation

@sid597
Copy link
Collaborator

@sid597 sid597 commented Oct 8, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Sidebar configuration now stays in sync when initial settings change, preventing stale layouts.
    • Clicking a section title without sub-items no longer triggers unintended navigation.
    • Chevron actions only apply when a section has sub-items, reducing accidental clicks.
  • Refactor

    • Simplified sidebar item click logic to make interactions more predictable and consistent.

@linear
Copy link

linear bot commented Oct 8, 2025

ENG-947 Bug report

@supabase
Copy link

supabase bot commented Oct 8, 2025

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 ↗︎.

Copy link
Collaborator Author

sid597 commented Oct 8, 2025

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

@sid597 sid597 changed the title remove open target call for personal section header ENG-947 dragging and/or clicking on section header crashes the sidebar Oct 8, 2025
@sid597 sid597 marked this pull request as ready for review October 8, 2025 10:10
@sid597 sid597 requested a review from mdroidian October 8, 2025 10:15
@mdroidian
Copy link
Contributor

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 12, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 12, 2025

📝 Walkthrough

Walkthrough

Updates adjust click handling in PersonalSectionItem to only toggle when children exist and remove the fallback open action. LeftSidebarView gains an effect to synchronize local config state with changes to initialConfig by invoking setConfig(initialConfig).

Changes

Cohort / File(s) Summary
Sidebar interactions and config sync
apps/roam/src/components/LeftSidebarView.tsx
PersonalSectionItem: title onClick now calls handleChevronClick only when children exist; otherwise no action. LeftSidebarView: added useEffect to update config state whenever initialConfig changes via setConfig(initialConfig).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant PSI as PersonalSectionItem
  participant LSV as LeftSidebarView
  participant State as Config State

  rect rgba(200,230,255,0.3)
    note over PSI: Title click behavior
    User->>PSI: Click title
    alt Has children
      PSI->>PSI: handleChevronClick()
    else No children
      PSI-->>User: No action
    end
  end

  rect rgba(220,255,220,0.3)
    note over LSV,State: Sync initialConfig -> config
    LSV->>LSV: useEffect(dep: initialConfig)
    LSV->>State: setConfig(initialConfig)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

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 clearly summarizes the main issue addressed by the changeset, stating that dragging or clicking a section header crashes the sidebar, which directly corresponds to the bug fix implemented. It focuses on the primary change without extraneous details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a69b94e and 37c4a1e.

📒 Files selected for processing (1)
  • apps/roam/src/components/LeftSidebarView.tsx (2 hunks)
🔇 Additional comments (2)
apps/roam/src/components/LeftSidebarView.tsx (2)

576-578: LGTM! State synchronization pattern is sound.

The effect correctly syncs local state with external config changes, supporting the use case described in the past comment (updating state when sections are added or moved). The pattern of maintaining local state for optimistic drag-drop updates while respecting external changes is appropriate.

Based on the past review comment from sid597.


221-225: Confirm leaf‐section navigation removal is intentional
Leaf sections no longer open on click and no alternative navigation exists—verify this UX change is intended.


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.

@sid597 sid597 requested a review from mdroidian October 19, 2025 18:00
@sid597 sid597 force-pushed the eng-947-bug-report branch from 0204406 to 54d3e6e Compare October 19, 2025 18:13
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

lfg

@sid597 sid597 merged commit a449ecd into main Oct 19, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this to Done in General Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants