-
Notifications
You must be signed in to change notification settings - Fork 3
ENG-1051 remove drag and drop until we find a good solution for it #544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENG-1051 remove drag and drop until we find a good solution for it #544
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis PR removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx (1)
210-217: Fix stale disablement of Folded toggle
disabled={!globalSection.children?.length}never re-evaluates after add/remove becauseglobalSectionstays at the snapshot captured duringinitialize(). After you add the first page, the Folded control is still greyed out, so users can’t enable it without reloading the settings pane. Drive the disable flag from the reactivepagesstate (or updateglobalSection.children) so the UI unlocks immediately.- disabled={!globalSection.children?.length} + disabled={!pages.length}
🧹 Nitpick comments (1)
apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx (1)
206-213: Skip the second refreshAndNotify() call
addChildToSectionalready invokesrefreshAndNotify(). Calling it again here forces a second config refresh and duplicate sidebar notifications. Dropping the extra call keeps the behaviour identical while avoiding redundant work.if (childInput && section.childrenUid) { await addChildToSection(section, section.childrenUid, childInput); setChildInput(""); setChildInputKey((prev) => prev + 1); - refreshAndNotify(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/roam/package.json(0 hunks)apps/roam/src/components/LeftSidebarView.tsx(2 hunks)apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx(3 hunks)apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/roam/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Prefertypeoverinterface
Use explicit return types for functions
Avoidanytypes when possible
Files:
apps/roam/src/components/LeftSidebarView.tsxapps/roam/src/components/settings/LeftSidebarGlobalSettings.tsxapps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx,js,jsx}: Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use Prettier with the project's configuration
Maintain consistent naming conventions: PascalCase for components and types
Maintain consistent naming conventions: camelCase for variables and functions
Maintain consistent naming conventions: UPPERCASE for constants
Files:
apps/roam/src/components/LeftSidebarView.tsxapps/roam/src/components/settings/LeftSidebarGlobalSettings.tsxapps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx
apps/roam/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{js,jsx,ts,tsx}: Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when working with the Roam Extension API
Files:
apps/roam/src/components/LeftSidebarView.tsxapps/roam/src/components/settings/LeftSidebarGlobalSettings.tsxapps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx
🧠 Learnings (4)
📚 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/LeftSidebarView.tsxapps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Applied to files:
apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when working with the Roam Extension API
Applied to files:
apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx
mdroidian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go ahead with this as I still experience issues with dnd: #545 (comment)
Also, let's make a new task to add up down (🔼🔽) arrows to each page/section in the settings panel so that they can be reordered.

Summary by CodeRabbit