saas and mobile fixes #1467
saas and mobile fixes #1467
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements fixes for SaaS mode configuration and mobile responsiveness. The changes disable SaaS mode in the dev environment and add conditional rendering throughout the UI to show/hide SaaS-specific features accordingly. Additionally, it introduces a table folder/category grouping feature in the table switcher and adds mobile-friendly UI adjustments for the saved filters panel.
Key changes:
- Disabled SaaS mode in development environment configuration
- Wrapped SaaS-specific UI elements (Google/GitHub login, SSO, registration) with
isSaasconditionals - Added table folder/category grouping in the table switcher dropdown
- Implemented mobile-responsive design for filter panels with collapsible comparator selectors
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/environments/environment.dev.ts | Changed saas flag from true to false to disable SaaS features in dev environment |
| frontend/src/app/components/login/login.component.ts | Wrapped Google login initialization in isSaas conditional check |
| frontend/src/app/components/login/login.component.html | Added isSaas checks to conditionally render Google login, GitHub login, divider, and SSO buttons |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.html | Added mobile-friendly comparator text display for text and number filters |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.css | Added responsive CSS media queries for mobile devices (≤600px) to hide dropdown selectors and show text labels |
| frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts | Added Folder interface, folder-related methods, and MatSelectModule import; replaced autocomplete with select dropdown |
| frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html | Replaced autocomplete table switcher with mat-select dropdown that supports folder grouping |
| frontend/src/app/components/dashboard/dashboard.component.ts | Added tableFolders property and loadTableFolders() method to fetch and transform table categories |
| frontend/src/app/components/dashboard/dashboard.component.html | Passed tableFolders to the table view component |
| frontend/src/app/components/company/company.component.html | Added isSaas check to custom domain display section |
| frontend/src/app/app.component.html | Added isSaas conditionals to registration and login header links |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public isAIpanelOpened: boolean = false; | ||
|
|
||
| public uiSettings: ConnectionSettingsUI; | ||
| public tableFolders: Folder[] = []; |
There was a problem hiding this comment.
The property name tableFolders could be more consistent with the API terminology. Since the backend API endpoint is /table-categories and uses the TableCategory interface, consider renaming to tableCategories to maintain consistency with the domain model throughout the codebase.
| export interface Folder { | ||
| id: string; | ||
| name: string; | ||
| tableIds: string[]; | ||
| } |
There was a problem hiding this comment.
The Folder interface is exported from this component file but is primarily a data model. Consider moving it to a dedicated models file (e.g., src/app/models/folder.ts or alongside TableCategory in src/app/models/connection.ts) to improve code organization and reusability, especially since it's imported by the dashboard component.
| getFolderTables(folder: Folder): TableProperties[] { | ||
| return this.tables.filter(table => folder.tableIds.includes(table.table)); | ||
| } | ||
|
|
||
| getUncategorizedTables(): TableProperties[] { | ||
| const categorizedTableIds = new Set<string>(); | ||
| this.folders.forEach(folder => { | ||
| folder.tableIds.forEach(id => categorizedTableIds.add(id)); | ||
| }); | ||
| return this.tables.filter(table => !categorizedTableIds.has(table.table)); | ||
| } |
There was a problem hiding this comment.
The getFolderTables and getUncategorizedTables methods should handle the case where this.tables could be undefined or null. Consider adding null checks to prevent potential runtime errors when these properties are not yet initialized.
| switchTable(tableName: string) { | ||
| if (tableName && tableName !== this.name) { | ||
| this.router.navigate([`/dashboard/${this.connectionID}/${tableName}`], { | ||
| queryParams: { page_index: 0, page_size: 30 } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| getFolderTables(folder: Folder): TableProperties[] { | ||
| return this.tables.filter(table => folder.tableIds.includes(table.table)); | ||
| } | ||
|
|
||
| getUncategorizedTables(): TableProperties[] { | ||
| const categorizedTableIds = new Set<string>(); | ||
| this.folders.forEach(folder => { | ||
| folder.tableIds.forEach(id => categorizedTableIds.add(id)); | ||
| }); | ||
| return this.tables.filter(table => !categorizedTableIds.has(table.table)); | ||
| } |
There was a problem hiding this comment.
The new methods switchTable, getFolderTables, and getUncategorizedTables lack test coverage. Since the component has a spec file with existing tests, please add tests for these new methods to verify they work correctly with folders and handle edge cases like empty folder arrays or uncategorized tables.
| private loadTableFolders() { | ||
| this._connections.getTablesFolders(this.connectionID).subscribe({ | ||
| next: (categories: TableCategory[]) => { | ||
| if (categories && categories.length > 0) { | ||
| this.tableFolders = categories.map(cat => ({ | ||
| id: cat.category_id, | ||
| name: cat.category_name, | ||
| tableIds: cat.tables | ||
| })); | ||
| } else { | ||
| this.tableFolders = []; | ||
| } | ||
| }, | ||
| error: (error) => { | ||
| console.error('Error fetching table folders:', error); | ||
| this.tableFolders = []; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
The new loadTableFolders method lacks test coverage. Since this component has a spec file with existing tests, please add tests to verify this method correctly handles successful API responses, error scenarios, and empty category arrays.
No description provided.