Conversation
There was a problem hiding this comment.
Pull request overview
Improves the “auto AI settings” UX by adding a prominent “Configure all” CTA in the dashboard and enhancing the auto-configure loading screen to allow users to proceed to tables while setup continues.
Changes:
- Adds a “Configure all” internal-link action to the dashboard’s
alert_settingsInfo. - Replaces the
alert_settingsInfo<app-alert>rendering with a custom “AI Configuration” alert block + new styles. - Updates auto-configure loading copy and adds an “Open tables” button + hint text.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/components/dashboard/db-tables-data-source.ts | Adds “Configure all” internal-link action to the settings info alert. |
| frontend/src/app/components/dashboard/dashboard.component.html | Introduces a custom AI configuration alert block in place of <app-alert> for alert_settingsInfo. |
| frontend/src/app/components/dashboard/dashboard.component.css | Adds custom CSS for the AI configuration alert block. |
| frontend/src/app/components/auto-configure/auto-configure.component.html | Updates loading UI copy; adds “Open tables” CTA and hint. |
| frontend/src/app/components/auto-configure/auto-configure.component.css | Styles the new CTA and hint text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.alert_settingsInfo = { | ||
| id: 10001, | ||
| type: AlertType.Info, | ||
| message: 'Configure now to reveal advanced table functionality and features.', | ||
| actions: [ | ||
| // { | ||
| // type: AlertActionType.Button, | ||
| // caption: 'AI generate', | ||
| // }, | ||
| { | ||
| type: AlertActionType.Link, | ||
| caption: 'Configure all', | ||
| to: `/auto-configure/${connectionID}` | ||
| }, |
There was a problem hiding this comment.
alert_settingsInfo now includes a “Configure all” internal link, but the dashboard template no longer renders alert_settingsInfo via <app-alert [alert]=...>, so these actions (and even the message) risk becoming dead/duplicated configuration. Consider either (a) continuing to render the alert object in the template and styling/customizing the alert component, or (b) changing alert_settingsInfo to a simple boolean/flag and moving the CTA/link text entirely into the dashboard UI so there’s a single source of truth.
| <div *ngIf="dataSource.alert_settingsInfo" class="ai-config-alert"> | ||
| <mat-icon class="ai-config-alert__icon">auto_awesome</mat-icon> | ||
| <div class="ai-config-alert__message mat-body-1"> | ||
| <strong class="ai-config-alert__title">New: AI Configuration</strong> | ||
| <p class="ai-config-alert__text">Automatically configure all your tables with AI. You can adjust changes in Settings and UI widgets.</p> | ||
| </div> | ||
| <div class="ai-config-alert__actions"> | ||
| <a mat-button [routerLink]="'/auto-configure/' + connectionID">Configure all</a> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
This block is gated by dataSource.alert_settingsInfo but ignores the alert object’s message and actions, which can lead to inconsistencies (e.g., db-tables-data-source.ts still defines “Settings”/“Fields display” actions). Either render <app-alert [alert]="dataSource.alert_settingsInfo"> and customize it, or add equivalent links here (especially since the text mentions “Settings and UI widgets” but only provides “Configure all”).
| <div *ngIf="dataSource.alert_settingsInfo" class="ai-config-alert"> | |
| <mat-icon class="ai-config-alert__icon">auto_awesome</mat-icon> | |
| <div class="ai-config-alert__message mat-body-1"> | |
| <strong class="ai-config-alert__title">New: AI Configuration</strong> | |
| <p class="ai-config-alert__text">Automatically configure all your tables with AI. You can adjust changes in Settings and UI widgets.</p> | |
| </div> | |
| <div class="ai-config-alert__actions"> | |
| <a mat-button [routerLink]="'/auto-configure/' + connectionID">Configure all</a> | |
| </div> | |
| </div> | |
| <app-alert *ngIf="dataSource.alert_settingsInfo" [alert]="dataSource.alert_settingsInfo"></app-alert> |
| .ai-config-alert { | ||
| display: flex; | ||
| gap: 8px; | ||
| background-color: color-mix(in hsl, var(--color-accentedPalette-500), transparent 95%); | ||
| backdrop-filter: blur(4px); | ||
| border-top: 1px solid var(--color-accentedPalette-500); | ||
| border-right: 1px solid var(--color-accentedPalette-500); | ||
| border-bottom: 1px solid var(--color-accentedPalette-500); | ||
| border-left: 12px solid var(--color-accentedPalette-500); | ||
| border-radius: 4px; | ||
| padding: 12px 16px 12px 8px; | ||
| } |
There was a problem hiding this comment.
The new .ai-config-alert styles substantially duplicate ui-components/alert/alert.component.css. This increases drift risk (future alert style tweaks won’t apply here). Consider reusing <app-alert> with message: null and projecting custom content, or refactoring shared alert styles into a common class/stylesheet used by both components.
| <h2 class="auto-configure__title">Setting up your dashboard</h2> | ||
| <p class="auto-configure__text">We're analyzing your database structure and applying the best configuration. This may take a while.</p> | ||
| <a mat-flat-button class="auto-configure__open-btn" [routerLink]="'/dashboard/' + connectionId()">Open tables</a> | ||
| <p class="auto-configure__hint">Setup will continue in the background</p> |
There was a problem hiding this comment.
Adding an “Open tables” navigation while loading() remains true means users can leave before _configure() resolves. The current component logic navigates to /dashboard/:id when the async call completes; without cancellation/teardown guarding, that navigation can still fire after the component is destroyed (potentially causing unexpected redirects or state resets). Also, connectionId() starts as null, so this routerLink can briefly become /dashboard/null; consider rendering/enabling the button only once connectionId() is set.
No description provided.