Conversation
…s true
Discourse context renders ResultsView with preventSavingSettings={true}
but the kebab menu still showed settings items (Layout, Alias, Hide
Interface, etc.). Clicking them wrote Roam blocks onto the page.
Hide the Results submenu, Alias, Hide Interface, and Copy Query items
when preventSavingSettings is true. Menu collapses to "Share Data" in
discourse context and overlays.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Pre-existing no-floating-promises warnings on setInputSetting / navigator.clipboard.writeText calls. The previous commit re-indented these lines due to JSX wrapping, so reviewdog now flags them as added. Mark them void — they're fire-and-forget side effects.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
📝 WalkthroughWalkthroughThe ResultsView component's "more" menu was restructured to consolidate multiple settings options (Layout, Column Views, Search, Inputs, Filters, Sort, Get Random, Rows per page) into a nested "Results" submenu, with conditional rendering of settings-based items controlled by Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Copy Query is read-only — it doesn't save settings, so gating it behind
preventSavingSettings was a misuse of the flag. It is still broken in
discourse context (copies whole page tree wrapped in {{query block}})
but that's a separate bug for a separate ticket.
Also restores the trailing Divider to unconditional since the only
reason it was conditional was the Copy Query wrap.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ onDragEnd in ResultsTable persists column widths even when preventSavingSettings is true (apps/roam/src/components/results-view/ResultsTable.tsx:622-629)
The PR forwards preventSavingSettings to ResultsTable and correctly guards resultHeaderSetFilters at ResultsTable.tsx:636, but onDragEnd at ResultsTable.tsx:622-629 unconditionally calls setInputSettings to persist column widths to Roam blocks. When preventSavingSettings is true, dragging column borders will still write layout width settings, defeating the purpose of the flag.
View 3 additional findings in Devin Review.
1 participant
https://www.loom.com/share/ca2fa22b6a9e419abe811f8df4d4c4b0
NOTE: For review use hide whitespaces settings
Summary by CodeRabbit