Conversation
…de toggle if no custom folders
There was a problem hiding this comment.
Pull request overview
Updates the dashboard tables sidebar to improve drag-and-drop behavior and make the “All tables” folder behave consistently when it’s the only folder.
Changes:
- Force the “All tables” folder to remain expanded when no custom folders exist, including during state preserve/restore.
- Prevent collapsing “All tables” when it’s the only folder.
- Disable table/folder drag-and-drop interactions for non-
editaccess levels and hide the expand icon when collapsing isn’t allowed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/app/components/dashboard/db-tables-list/db-tables-list.component.ts | Adds logic to keep “All tables” expanded and prevents collapsing it when it’s the only folder; updates custom-folder detection to use the folder id. |
| frontend/src/app/components/dashboard/db-tables-list/db-tables-list.component.html | Gates drag/drop handlers and draggable state by access level; conditionally hides the expand/collapse icon for “All tables”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If no custom folders exist, always keep "All tables" expanded | ||
| if (!this.hasCustomFolders()) { | ||
| const allTablesFolder = this.folders.find(folder => folder.id === 'all-tables-kitten'); | ||
| if (allTablesFolder) { | ||
| allTablesFolder.expanded = true; | ||
| } |
There was a problem hiding this comment.
The special folder id 'all-tables-kitten' is now referenced in several places (ngOnInit, toggleFolder, hasCustomFolders, preserve/restore). Consider extracting it into a single constant (e.g., readonly field) to avoid accidental mismatches and make future refactors safer.
| [class.drag-over]="accessLevel === 'edit' && dragOverFolder !== 'all-tables-kitten' && dragOverFolder === folder.id" | ||
| (dragover)="accessLevel === 'edit' && onFolderDragOver($event, folder.id)" | ||
| (dragleave)="accessLevel === 'edit' && onFolderDragLeave($event, folder.id)" | ||
| (drop)="accessLevel === 'edit' && onFolderDrop($event, folder)"> |
There was a problem hiding this comment.
Drag/drop is now gated in the template via accessLevel === 'edit', but the underlying handlers (e.g., drop/dragover) still mutate folder state and call saveFolders() without checking permissions. Add an early return in the component methods when accessLevel !== 'edit' so the permission rule is enforced even if these handlers are invoked from another template change, a test harness, or programmatically.
| // Prevent collapsing "All tables" when no custom folders exist | ||
| if (folder.id === 'all-tables-kitten' && folder.expanded && !this.hasCustomFolders()) { | ||
| return; | ||
| } | ||
| folder.expanded = !folder.expanded; |
There was a problem hiding this comment.
This change introduces new behavior around folder expansion (forcing “All tables” expanded when it’s the only folder, and preventing collapse in toggleFolder). Please add/update unit tests to cover these cases so regressions in sidebar state preservation and custom-folder detection are caught.
No description provided.