Add column hiding functionality #306
Conversation
Co-authored-by: mclicks <[email protected]>
WalkthroughThe changes introduce dynamic column hiding functionality to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ResultsView
participant SettingsStore
User->>ResultsView: Clicks "Hide Columns" menu item
ResultsView->>User: Shows column visibility panel
User->>ResultsView: Toggles column visibility switches
ResultsView->>ResultsView: Updates hiddenColumns state
ResultsView->>SettingsStore: Persists hiddenColumns changes
ResultsView->>User: Updates visible columns in UI
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/ResultsView.tsx(17 hunks)src/utils/parseResultSettings.ts(2 hunks)
🔇 Additional comments (6)
src/utils/parseResultSettings.ts (1)
117-120: Clean implementation of hidden columns parsingThe implementation correctly follows the existing pattern for parsing settings from the result node. The extraction of hidden columns is straightforward and properly integrated into the returned settings object.
Also applies to: 168-168
src/components/ResultsView.tsx (5)
317-323: Well-implemented state management for column visibilityGood use of
useMemoto compute visible columns efficiently. The dependency array correctly includes bothcolumnsandhiddenColumnsto ensure proper recomputation when either changes.
415-430: Correct implementation of hidden columns persistenceThe function properly handles state updates and persistence, following the established pattern used elsewhere in the component for saving settings to Roam blocks.
484-484: Thorough update of column references to use visible columnsExcellent job systematically replacing
columnswithvisibleColumnsthroughout the component. All the changes are appropriate for their contexts - export, UI selections, and layout rendering.Also applies to: 603-603, 682-684, 691-691, 717-717, 853-853, 1191-1191, 1213-1213, 1219-1219, 1228-1228
912-912: View settings for hidden columns are preservedThe filtering ensures that view settings are only displayed for visible columns, which is correct. Note that view settings for hidden columns are preserved in state and will reappear when columns are unhidden.
381-382: Well-integrated menu item with appropriate visual indicatorsGood implementation details:
- The dirty indicator on both the main menu icon and the "Hide Columns" menu item helps users understand when columns are hidden
- The
eye-officon is an intuitive choice for this featureAlso applies to: 1028-1035
| ) : isEditHiddenColumns ? ( | ||
| <div className="relative p-4" style={{ minWidth: "320px" }}> | ||
| <MenuHeading | ||
| onClear={() => setIsEditHiddenColumns(false)} | ||
| text="Hide Columns" | ||
| /> | ||
| <div className="flex flex-col gap-2 py-2"> | ||
| {columns.map(({ key }) => ( | ||
| <div key={key} className="flex items-center justify-between"> | ||
| <span className="text-sm">{key}</span> | ||
| <Switch | ||
| checked={!hiddenColumns.includes(key)} | ||
| onChange={(e) => { | ||
| const isVisible = (e.target as HTMLInputElement).checked; | ||
| if (isVisible) { | ||
| onHiddenColumnsChange(hiddenColumns.filter(col => col !== key)); | ||
| } else { | ||
| onHiddenColumnsChange([...hiddenColumns, key]); | ||
| } | ||
| }} | ||
| /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider preventing users from hiding all columns
The implementation looks good, but there's an edge case where users could potentially hide all columns, leaving the results view empty. Consider adding validation to ensure at least one column remains visible.
onChange={(e) => {
const isVisible = (e.target as HTMLInputElement).checked;
if (isVisible) {
onHiddenColumnsChange(hiddenColumns.filter(col => col !== key));
} else {
+ // Prevent hiding the last visible column
+ if (hiddenColumns.length >= columns.length - 1) {
+ renderToast({
+ id: "hide-columns-error",
+ content: "At least one column must remain visible",
+ intent: Intent.WARNING,
+ });
+ return;
+ }
onHiddenColumnsChange([...hiddenColumns, key]);
}
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ) : isEditHiddenColumns ? ( | |
| <div className="relative p-4" style={{ minWidth: "320px" }}> | |
| <MenuHeading | |
| onClear={() => setIsEditHiddenColumns(false)} | |
| text="Hide Columns" | |
| /> | |
| <div className="flex flex-col gap-2 py-2"> | |
| {columns.map(({ key }) => ( | |
| <div key={key} className="flex items-center justify-between"> | |
| <span className="text-sm">{key}</span> | |
| <Switch | |
| checked={!hiddenColumns.includes(key)} | |
| onChange={(e) => { | |
| const isVisible = (e.target as HTMLInputElement).checked; | |
| if (isVisible) { | |
| onHiddenColumnsChange(hiddenColumns.filter(col => col !== key)); | |
| } else { | |
| onHiddenColumnsChange([...hiddenColumns, key]); | |
| } | |
| }} | |
| /> | |
| </div> | |
| ))} | |
| </div> | |
| </div> | |
| ) : isEditHiddenColumns ? ( | |
| <div className="relative p-4" style={{ minWidth: "320px" }}> | |
| <MenuHeading | |
| onClear={() => setIsEditHiddenColumns(false)} | |
| text="Hide Columns" | |
| /> | |
| <div className="flex flex-col gap-2 py-2"> | |
| {columns.map(({ key }) => ( | |
| <div key={key} className="flex items-center justify-between"> | |
| <span className="text-sm">{key}</span> | |
| <Switch | |
| checked={!hiddenColumns.includes(key)} | |
| onChange={(e) => { | |
| const isVisible = (e.target as HTMLInputElement).checked; | |
| if (isVisible) { | |
| onHiddenColumnsChange(hiddenColumns.filter(col => col !== key)); | |
| } else { | |
| // Prevent hiding the last visible column | |
| if (hiddenColumns.length >= columns.length - 1) { | |
| renderToast({ | |
| id: "hide-columns-error", | |
| content: "At least one column must remain visible", | |
| intent: Intent.WARNING, | |
| }); | |
| return; | |
| } | |
| onHiddenColumnsChange([...hiddenColumns, key]); | |
| } | |
| }} | |
| /> | |
| </div> | |
| ))} | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In src/components/ResultsView.tsx around lines 970 to 994, the current code
allows users to hide all columns, which can result in an empty results view. To
fix this, add validation in the onChange handler of the Switch component to
prevent hiding the last visible column. Specifically, check if hiding the
current column would leave no visible columns, and if so, do not update
hiddenColumns state to ensure at least one column remains visible.
Add the ability to hide columns in the results view, with settings persisted as blocks.
Summary by CodeRabbit
New Features
Improvements