Refactor connections list with hosted PostgreSQL banner#1685
Conversation
… and demo sections Split the connections page into three visually distinct sections: - Hosted PostgreSQL as a standalone banner with gradient card - Connect your own as a responsive DB grid - Demo panels as a 3-column grid with section labels Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add separate dark mode gradient to keep the deep tones, while using a brighter indigo palette for light theme. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaces many CSS classes and restructures demo/own connections templates from Angular structural directives ( Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Connections List UI to promote the hosted PostgreSQL option as a standalone banner and reorganizes “connect your own” + demo connections into grid-based sections for improved responsiveness and theming.
Changes:
- Introduces a full-width “Hosted PostgreSQL” banner section (light/dark gradient + “Recommended” badge).
- Moves supported DB types into a responsive “CONNECT YOUR OWN” grid.
- Updates demo connections into a 3-column grid with a section label.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| frontend/src/app/components/connections-list/own-connections/own-connections.component.html | Reorders layout into sections, adds hosted banner + DB grid, updates connection list rendering to new control-flow syntax. |
| frontend/src/app/components/connections-list/own-connections/own-connections.component.css | Adds styling for section labels, hosted banner, DB grid/cards; reorganizes existing styles to match new layout. |
| frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html | Converts demo connections list into a grid layout and changes heading/label rendering. |
| frontend/src/app/components/connections-list/demo-connections/demo-connections.component.css | Adds grid/card styles for the new demo layout and introduces a local section__label style. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| alt="Rocketadmin connection"> | ||
| } | ||
| <div class="connectionInfo"> | ||
| <h2 class="connectionInfo__connectionTitle">{{ $any(connectionItem).displayTitle }}</h2> |
There was a problem hiding this comment.
Using $any(connectionItem).displayTitle bypasses template type-checking. Since displayTitle is a known UI field added in the connections flow, consider updating the connections input type to a view-model interface that includes displayTitle (or extend ConnectionItem) so the template can stay type-safe without $any.
| <h2 class="connectionInfo__connectionTitle">{{ $any(connectionItem).displayTitle }}</h2> | |
| <h2 class="connectionInfo__connectionTitle">{{ connectionItem.displayTitle }}</h2> |
| <div class="demoCard__info"> | ||
| <strong>{{ $any(testConnectionItem).displayTitle }}</strong> | ||
| <span class="demoCard__type">{{ testDatabasesNames[testConnectionItem.connection.type] }}</span> |
There was a problem hiding this comment.
$any(testConnectionItem).displayTitle disables template type-checking. Consider changing testConnections to a typed view-model that includes displayTitle (or extend ConnectionItem) so this stays type-safe.
| color="primary" | ||
| class="fabHostedButton" | ||
| class="hosted-banner" | ||
| data-testid="create-hosted-database-button" |
There was a problem hiding this comment.
The hosted DB CTA test selector changed: unit tests still query for data-testid="create-hosted-database-empty-button" (see own-connections.component.spec.ts). Either keep the previous data-testid as an alias on this button, or update the tests to the new selector to avoid CI failures.
| data-testid="create-hosted-database-button" | |
| data-testid="create-hosted-database-empty-button" | |
| data-testid-new="create-hosted-database-button" |
| @if (showHostedDatabaseEntry) { | ||
| <button | ||
| mat-stroked-button | ||
| type="button" | ||
| color="primary" | ||
| class="fabHostedButton" | ||
| data-testid="create-hosted-database-fab-button" | ||
| [disabled]="creatingHostedDatabase()" | ||
| (click)="createHostedDatabase()"> | ||
| <mat-icon>cloud_queue</mat-icon> | ||
| {{ creatingHostedDatabase() ? 'Creating...' : 'Create hosted PostgreSQL' }} | ||
| </button> |
There was a problem hiding this comment.
When showHostedDatabaseEntry is true and there are existing connections, the UI will render two CTAs that both call createHostedDatabase() (the full-width hosted banner and this FAB button). This duplication can confuse users and makes analytics/QA harder; consider showing only one (e.g., hide the FAB hosted button when the banner is present, or restrict the FAB to mobile-only via an explicit condition).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
frontend/src/app/components/connections-list/own-connections/own-connections.component.html (2)
75-75: Consider using index for tracking if database order may change.The
track supportedDatabaseexpression works by tracking the string value directly. This is fine ifsupportedOrderedDatabasesis a static list, but if items could be reordered dynamically, consider usingtrack $indexor a stable identifier property for optimal change detection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.html` at line 75, The current ngFor uses "track supportedDatabase" which tracks by the string value and may mis-handle reorderings; update the ngFor in own-connections.component.html to track by a stable identifier or index instead—e.g. use "track $index" if order changes frequently or use a unique property on each item (e.g. supportedDatabase.id or supportedDatabase.key) so change detection in the loop over supportedOrderedDatabases correctly preserves item identity; locate the template line containing "for (supportedDatabase of supportedOrderedDatabases; track supportedDatabase)" and replace the track expression accordingly.
20-21: Update type interfaces to includeconnection_propertiesanddisplayTitleproperties for compile-time type checking.The
$any()casts forconnection_properties?.logo_url(lines 20-21) anddisplayTitle(line 30) indicate these properties aren't defined in the type interfaces. Theconnection_propertiesobject is returned by the backend API but not typed in theConnectioninterface, anddisplayTitleis dynamically added by the service layer but not declared in theConnectionIteminterface. Consider updating these interfaces to include these properties and remove the$any()casts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.html` around lines 20 - 21, Update the TypeScript interfaces to declare the missing properties so you can remove the $any() casts: add a connection_properties field (e.g. connection_properties?: { logo_url?: string; /* add other backend fields */ }) to the Connection interface and add displayTitle?: string to the ConnectionItem interface (or the exact type that stores items returned to the component); also update the service method that assigns displayTitle to return the properly typed ConnectionItem so the template can reference displayTitle and connection_properties.logo_url without using $any().frontend/src/app/components/connections-list/own-connections/own-connections.component.css (1)
356-356: Avoid using!importantfor height.Using
!importanton line 356 (and line 362) can make future maintenance difficult. If this is needed to override Material button styles, consider using a more specific selector instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.css` at line 356, The CSS rule in own-connections component uses height: 48px !important which should be replaced by a more specific selector instead of using !important; locate the selector applied (the rule currently forcing height: 48px on the button within OwnConnectionsComponent) and remove the !important, then increase selector specificity (for example target the component host and the exact Material button classes such as .own-connections-component .mat-button.mat-flat-button or a custom wrapper class) so the height: 48px applies without !important and keeps Material styles overridden only for this component.frontend/src/app/components/connections-list/demo-connections/demo-connections.component.css (1)
7-15: Consider extracting shared.section__labelstyles.The
.section__labelstyling is duplicated between this file andown-connections.component.css. If these components share a parent module or can access shared styles, consider extracting this to a common stylesheet to maintain DRY principles.Also applies to: 20-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/demo-connections/demo-connections.component.css` around lines 7 - 15, The .section__label rule is duplicated between demo-connections and own-connections; extract this selector into a shared stylesheet (e.g., a module-level or global shared CSS/SCSS file) and import that shared file into both components so they reuse the single definition; remove the duplicate .section__label blocks from demo-connections.component.css and own-connections.component.css and verify the selector and properties (font-size, font-weight, letter-spacing, text-transform, opacity, margin-bottom) are preserved and still apply without changing specificity.frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html (1)
20-20: AdddisplayTitleproperty to theConnectionIteminterface for proper type safety.The
displayTitleproperty is added at runtime inconnections.service.tsbut is not defined in theConnectionIteminterface, which is why$any()is required here. Update the interface to includedisplayTitle: stringto enable proper type checking in the template.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html` at line 20, The template uses testConnectionItem.displayTitle but the ConnectionItem interface lacks that property, forcing the template to use $any(); add displayTitle: string to the ConnectionItem interface declaration so the runtime-added property from connections.service.ts is reflected in types, then remove the need for $any() in demo-connections.component.html; update the interface (symbol: ConnectionItem) and ensure any factory/mapper in connections.service.ts that sets displayTitle still produces a string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`:
- Line 160: Replace the deprecated CSS property grid-gap with the standard gap
in the same rule: locate the declaration "grid-gap: 20px;" and change it to
"gap: 20px;" (remove any duplicate/older grid-gap entries) so the layout uses
the modern gap property.
- Around line 42-46: The background shorthand is currently declared after
background-clip in own-connections.component.css which resets background-clip;
move the background-clip: padding-box declaration to come after the background:
... declaration (i.e., ensure background-clip appears below the background
shorthand) so the padding-box value isn't overridden.
---
Nitpick comments:
In
`@frontend/src/app/components/connections-list/demo-connections/demo-connections.component.css`:
- Around line 7-15: The .section__label rule is duplicated between
demo-connections and own-connections; extract this selector into a shared
stylesheet (e.g., a module-level or global shared CSS/SCSS file) and import that
shared file into both components so they reuse the single definition; remove the
duplicate .section__label blocks from demo-connections.component.css and
own-connections.component.css and verify the selector and properties (font-size,
font-weight, letter-spacing, text-transform, opacity, margin-bottom) are
preserved and still apply without changing specificity.
In
`@frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html`:
- Line 20: The template uses testConnectionItem.displayTitle but the
ConnectionItem interface lacks that property, forcing the template to use
$any(); add displayTitle: string to the ConnectionItem interface declaration so
the runtime-added property from connections.service.ts is reflected in types,
then remove the need for $any() in demo-connections.component.html; update the
interface (symbol: ConnectionItem) and ensure any factory/mapper in
connections.service.ts that sets displayTitle still produces a string.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`:
- Line 356: The CSS rule in own-connections component uses height: 48px
!important which should be replaced by a more specific selector instead of using
!important; locate the selector applied (the rule currently forcing height: 48px
on the button within OwnConnectionsComponent) and remove the !important, then
increase selector specificity (for example target the component host and the
exact Material button classes such as .own-connections-component
.mat-button.mat-flat-button or a custom wrapper class) so the height: 48px
applies without !important and keeps Material styles overridden only for this
component.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.html`:
- Line 75: The current ngFor uses "track supportedDatabase" which tracks by the
string value and may mis-handle reorderings; update the ngFor in
own-connections.component.html to track by a stable identifier or index
instead—e.g. use "track $index" if order changes frequently or use a unique
property on each item (e.g. supportedDatabase.id or supportedDatabase.key) so
change detection in the loop over supportedOrderedDatabases correctly preserves
item identity; locate the template line containing "for (supportedDatabase of
supportedOrderedDatabases; track supportedDatabase)" and replace the track
expression accordingly.
- Around line 20-21: Update the TypeScript interfaces to declare the missing
properties so you can remove the $any() casts: add a connection_properties field
(e.g. connection_properties?: { logo_url?: string; /* add other backend fields
*/ }) to the Connection interface and add displayTitle?: string to the
ConnectionItem interface (or the exact type that stores items returned to the
component); also update the service method that assigns displayTitle to return
the properly typed ConnectionItem so the template can reference displayTitle and
connection_properties.logo_url without using $any().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5820d8f6-48f4-4fe7-a9dd-2a5428af412b
📒 Files selected for processing (4)
frontend/src/app/components/connections-list/demo-connections/demo-connections.component.cssfrontend/src/app/components/connections-list/demo-connections/demo-connections.component.htmlfrontend/src/app/components/connections-list/own-connections/own-connections.component.cssfrontend/src/app/components/connections-list/own-connections/own-connections.component.html
frontend/src/app/components/connections-list/own-connections/own-connections.component.css
Outdated
Show resolved
Hide resolved
| display: grid; | ||
| grid-template-columns: repeat(auto-fit, minmax(142px, 1fr)); | ||
| align-items: stretch; | ||
| grid-gap: 20px; |
There was a problem hiding this comment.
Replace deprecated grid-gap with gap.
The grid-gap property is deprecated. Use the standard gap property instead for better forward compatibility.
🔧 Proposed fix
.dbGrid {
list-style: none;
display: grid;
grid-template-columns: repeat(auto-fit, minmax(142px, 1fr));
align-items: stretch;
- grid-gap: 20px;
+ gap: 20px;
width: 100%;🧰 Tools
🪛 Stylelint (17.4.0)
[error] 160-160: Expected "grid-gap" to be "gap" (property-no-deprecated)
(property-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`
at line 160, Replace the deprecated CSS property grid-gap with the standard gap
in the same rule: locate the declaration "grid-gap: 20px;" and change it to
"gap: 20px;" (remove any duplicate/older grid-gap entries) so the layout uses
the modern gap property.
There was a problem hiding this comment.
🧹 Nitpick comments (5)
frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html (1)
20-20: Consider addingdisplayTitleto theConnectionItemtype definition.Using
$any()to accessdisplayTitlebypasses TypeScript's type checking. IfdisplayTitleis a computed or enriched property, consider extending theConnectionIteminterface or creating a view model type that includes it. This would improve type safety and IDE support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html` at line 20, The template uses $any(testConnectionItem).displayTitle which bypasses TypeScript; add displayTitle to the ConnectionItem type (or create a ConnectionViewModel type) and update the component to expose items typed as ConnectionItem | ConnectionViewModel so the template can bind to testConnectionItem.displayTitle without $any; specifically modify the ConnectionItem interface (or add a new interface) to include displayTitle (string) and adjust any mapping logic in the component class that produces testConnectionItem to populate this field.frontend/src/app/components/connections-list/own-connections/own-connections.component.css (2)
350-363: Verify!importantnecessity for Material button height overrides.The
height: 48px !importantdeclarations on.fabAddButtonand.fabHostedButtonare likely needed to override Angular Material's default button heights. This is acceptable, but consider documenting why!importantis required here (e.g., via a CSS comment) for future maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.css` around lines 350 - 363, Add a brief CSS comment above the .fabAddButton and .fabHostedButton rules explaining why height: 48px !important is required (e.g., to override Angular Material button min-height/line-height defaults for this FAB-style button) so future maintainers understand the override; keep the !important in .fabAddButton and .fabHostedButton and ensure the comment references the selectors .fabAddButton and .fabHostedButton and mentions Angular Material as the source of the competing styles.
400-406: Consider reducing!importantusage if possible.Multiple
!importantdeclarations on.empty-state__textsuggest conflicts with Material typography classes. If these overrides are necessary due to Material's specificity, this is acceptable. Otherwise, consider increasing selector specificity instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.css` around lines 400 - 406, The CSS rule for .empty-state__text uses multiple !important flags which likely mask Material typography and create maintenance issues; remove unnecessary !important declarations and instead increase selector specificity (e.g., scope it under the component root class or use the component's host selector such as .own-connections .empty-state__text) or move the declaration to a stylesheet loaded after Material so it naturally overrides the library styles; keep !important only if after testing you confirm specificity cannot be resolved without it.frontend/src/app/components/connections-list/own-connections/own-connections.component.html (2)
5-5: Consider extracting the admin role check to a component property.The role check
currentUser?.role === 'ADMIN' || currentUser?.role === 'DB_ADMIN'appears at lines 5 and 112. Consider extracting this to a getter or computed property (e.g.,isAdminUser) for better maintainability and to ensure consistency if the role logic changes.get isAdminUser(): boolean { return this.currentUser?.role === 'ADMIN' || this.currentUser?.role === 'DB_ADMIN'; }Also applies to: 112-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.html` at line 5, Extract the repeated role check into a boolean getter (e.g., isAdminUser) on the OwnConnectionsComponent class that returns this.currentUser?.role === 'ADMIN' || this.currentUser?.role === 'DB_ADMIN', then update both template expressions that currently use currentUser?.role === 'ADMIN' || currentUser?.role === 'DB_ADMIN' to use isAdminUser instead; implement the getter in the component class (OwnConnectionsComponent) so all role logic is centralized and the template references become simpler and consistent.
133-133: Consider moving inline style to CSS class.The inline
style="text-align: center;"could be consolidated into the existing.empty-state__textclass or a sibling class for consistency with the component's styling approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.html` at line 133, The paragraph element in own-connections.component.html uses an inline style (style="text-align: center;"); remove that inline style and instead add the centering rule to the component stylesheet by either adding text-align: center to the existing .empty-state__text selector or creating a new modifier class (e.g., .empty-state__text--center) and applying that class alongside mat-body-1 in the template so styles remain consistent and component-scoped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/src/app/components/connections-list/demo-connections/demo-connections.component.html`:
- Line 20: The template uses $any(testConnectionItem).displayTitle which
bypasses TypeScript; add displayTitle to the ConnectionItem type (or create a
ConnectionViewModel type) and update the component to expose items typed as
ConnectionItem | ConnectionViewModel so the template can bind to
testConnectionItem.displayTitle without $any; specifically modify the
ConnectionItem interface (or add a new interface) to include displayTitle
(string) and adjust any mapping logic in the component class that produces
testConnectionItem to populate this field.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`:
- Around line 350-363: Add a brief CSS comment above the .fabAddButton and
.fabHostedButton rules explaining why height: 48px !important is required (e.g.,
to override Angular Material button min-height/line-height defaults for this
FAB-style button) so future maintainers understand the override; keep the
!important in .fabAddButton and .fabHostedButton and ensure the comment
references the selectors .fabAddButton and .fabHostedButton and mentions Angular
Material as the source of the competing styles.
- Around line 400-406: The CSS rule for .empty-state__text uses multiple
!important flags which likely mask Material typography and create maintenance
issues; remove unnecessary !important declarations and instead increase selector
specificity (e.g., scope it under the component root class or use the
component's host selector such as .own-connections .empty-state__text) or move
the declaration to a stylesheet loaded after Material so it naturally overrides
the library styles; keep !important only if after testing you confirm
specificity cannot be resolved without it.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.html`:
- Line 5: Extract the repeated role check into a boolean getter (e.g.,
isAdminUser) on the OwnConnectionsComponent class that returns
this.currentUser?.role === 'ADMIN' || this.currentUser?.role === 'DB_ADMIN',
then update both template expressions that currently use currentUser?.role ===
'ADMIN' || currentUser?.role === 'DB_ADMIN' to use isAdminUser instead;
implement the getter in the component class (OwnConnectionsComponent) so all
role logic is centralized and the template references become simpler and
consistent.
- Line 133: The paragraph element in own-connections.component.html uses an
inline style (style="text-align: center;"); remove that inline style and instead
add the centering rule to the component stylesheet by either adding text-align:
center to the existing .empty-state__text selector or creating a new modifier
class (e.g., .empty-state__text--center) and applying that class alongside
mat-body-1 in the template so styles remain consistent and component-scoped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25f8aeec-d819-4ccd-a92b-2587086fb847
📒 Files selected for processing (3)
frontend/src/app/components/connections-list/demo-connections/demo-connections.component.htmlfrontend/src/app/components/connections-list/own-connections/own-connections.component.cssfrontend/src/app/components/connections-list/own-connections/own-connections.component.html
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/src/app/components/connections-list/own-connections/own-connections.component.css (1)
49-54: Give the new interactive cards a visible:focus-visiblestate.
hosted-banneranddbCardonly define custom hover affordances right now. Mirroring those styles on:focus-visiblekeeps the new button/cards discoverable for keyboard users.♿ Suggested follow-up
-.hosted-banner:hover { +.hosted-banner:hover, +.hosted-banner:focus-visible { border-color: var(--color-accentedPalette-300); box-shadow: 0px 8px 24px var(--color-accentedPalette-100), 0px 1px 5px var(--color-accentedPalette-50); } @@ - .hosted-banner:hover { + .hosted-banner:hover, + .hosted-banner:focus-visible { border-color: var(--color-accentedPalette-500); box-shadow: 0px 8px 24px var(--color-accentedPalette-800), 0px 1px 5px var(--color-accentedPalette-600); } @@ -.dbCard:hover { +.dbCard:hover, +.dbCard:focus-visible { box-shadow: 0px 1px 5px 0px rgba(0, 0, 0, 0.2), 0px 3px 4px 0px rgba(0, 0, 0, 0.12), 0px 2px 4px 0px rgba(0, 0, 0, 0.14); } @@ - .dbCard:hover { + .dbCard:hover, + .dbCard:focus-visible { background: var(--surface-dark-color); border: 1px solid rgba(255, 255, 255, 0.25); box-shadow: 0px 1px 3px 0px rgba(255, 255, 255, 0.1), 0px 2px 2px 0px rgba(255, 255, 255, 0.08), 0px 0px 2px 0px rgba(255, 255, 255, 0.12); }Also applies to: 63-68, 194-199, 207-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.css` around lines 49 - 54, Add matching :focus-visible rules so interactive cards have the same visible focus affordances as hover; for .hosted-banner and the db card selectors referenced (e.g., .dbCard or similar classes in the file), duplicate the border-color and box-shadow styles used in the :hover state into a :focus-visible block, ensure any existing outline is handled (keep or replace with the same visual ring) and include keyboard-focus-friendly styling so keyboard users see the same highlight as mouse hover.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`:
- Around line 43-44: The hosted banner rules currently set flat background-color
values; update the hosted banner CSS in own-connections.component.css to use
linear-gradient instead of background-color and provide both light and dark
indigo gradient variants (use a default/light gradient and a dark variant under
a .dark selector or `@media` (prefers-color-scheme: dark)); replace the flat color
declarations (var(--color-accentedPalette-50) and
var(--color-accentedPalette-900)) with appropriate background:
linear-gradient(...) values and keep the text color rule intact, and make the
same change for the other occurrence around lines 58-60 so both banner variants
render the requested gradient treatment.
- Around line 24-27: Remove the duplicate text-transform declaration in
own-connections.component.css: keep a single text-transform: uppercase; (remove
the repeated line), leaving opacity: 0.45; and margin-bottom: 12px; unchanged so
Stylelint no longer flags duplicate properties.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.html`:
- Line 65: The badge text in the OwnConnections component template is incorrect:
in own-connections.component.html the span with class "hosted-banner__badge"
currently contains "Quickstart" but should read "Recommended" per the PR
requirement; update the inner text of that span (hosted-banner__badge) to
"Recommended" so the badge copy matches the spec.
- Line 45: The template currently renders the empty-state CTA in the `@else`
branch even when the component's connections model is null (loading); update the
conditional so the hosted/connect-your-own CTA only renders when connections is
not null and empty (e.g., replace the bare `@else` with an explicit check like
connections !== null && connections.length === 0 or wrap the CTA block with
*ngIf="connections !== null && connections.length === 0"); refer to the
connections variable and the `@else` branch in own-connections.component.html to
locate and update the condition.
---
Nitpick comments:
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`:
- Around line 49-54: Add matching :focus-visible rules so interactive cards have
the same visible focus affordances as hover; for .hosted-banner and the db card
selectors referenced (e.g., .dbCard or similar classes in the file), duplicate
the border-color and box-shadow styles used in the :hover state into a
:focus-visible block, ensure any existing outline is handled (keep or replace
with the same visual ring) and include keyboard-focus-friendly styling so
keyboard users see the same highlight as mouse hover.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8fe33e9-1a97-4e6f-bfcd-f7886d8b5d29
📒 Files selected for processing (2)
frontend/src/app/components/connections-list/own-connections/own-connections.component.cssfrontend/src/app/components/connections-list/own-connections/own-connections.component.html
| text-transform: uppercase; | ||
| opacity: 0.45; | ||
| margin-bottom: 12px; | ||
| text-transform: uppercase; |
There was a problem hiding this comment.
Remove the duplicate text-transform declaration.
Stylelint will keep flagging this rule until one of the duplicate declarations is removed.
🔧 Minimal fix
.section__label {
display: block;
font-size: 11px;
font-weight: 700;
letter-spacing: 0.08em;
text-transform: uppercase;
opacity: 0.45;
margin-bottom: 12px;
- text-transform: uppercase;
}📝 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.
| text-transform: uppercase; | |
| opacity: 0.45; | |
| margin-bottom: 12px; | |
| text-transform: uppercase; | |
| .section__label { | |
| display: block; | |
| font-size: 11px; | |
| font-weight: 700; | |
| letter-spacing: 0.08em; | |
| text-transform: uppercase; | |
| opacity: 0.45; | |
| margin-bottom: 12px; | |
| } |
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 24-24: Unexpected duplicate "text-transform" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`
around lines 24 - 27, Remove the duplicate text-transform declaration in
own-connections.component.css: keep a single text-transform: uppercase; (remove
the repeated line), leaving opacity: 0.45; and margin-bottom: 12px; unchanged so
Stylelint no longer flags duplicate properties.
| background-color: var(--color-accentedPalette-50); | ||
| color: var(--color-accentedPalette-900); |
There was a problem hiding this comment.
The hosted banner lost its promised gradient treatment.
The PR scope calls for light and dark indigo gradient variants, but these rules only set flat background-color values. That ships a different banner treatment than requested.
Also applies to: 58-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`
around lines 43 - 44, The hosted banner rules currently set flat
background-color values; update the hosted banner CSS in
own-connections.component.css to use linear-gradient instead of background-color
and provide both light and dark indigo gradient variants (use a default/light
gradient and a dark variant under a .dark selector or `@media`
(prefers-color-scheme: dark)); replace the flat color declarations
(var(--color-accentedPalette-50) and var(--color-accentedPalette-900)) with
appropriate background: linear-gradient(...) values and keep the text color rule
intact, and make the same change for the other occurrence around lines 58-60 so
both banner variants render the requested gradient treatment.
| @if (displayedCardCount > 3) { | ||
| <button type="button" mat-button color="accent" class="showAllButton" (click)="showLess()">Show less</button> | ||
| } | ||
| } @else { |
There was a problem hiding this comment.
Don't render the empty state while connections is still loading.
Line 2 and Line 128 already treat connections === null as a loading state, but this @else branch still renders the hosted/connect-your-own CTA when the input is null. That will flash the wrong state before the real list arrives.
🔧 Minimal fix
-} `@else` {
+} `@else` if (connections !== null) {📝 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.
| } @else { | |
| } `@else` if (connections !== null) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.html`
at line 45, The template currently renders the empty-state CTA in the `@else`
branch even when the component's connections model is null (loading); update the
conditional so the hosted/connect-your-own CTA only renders when connections is
not null and empty (e.g., replace the bare `@else` with an explicit check like
connections !== null && connections.length === 0 or wrap the CTA block with
*ngIf="connections !== null && connections.length === 0"); refer to the
connections variable and the `@else` branch in own-connections.component.html to
locate and update the condition.
| </span> | ||
| <span class="hosted-banner__subtitle">Provision a managed database in one click</span> | ||
| </div> | ||
| <span class="hosted-banner__badge">✦ Quickstart</span> |
There was a problem hiding this comment.
The badge copy doesn't match the PR requirement.
The scope calls for a "Recommended" badge, but Line 65 renders "Quickstart".
🔧 Copy fix
- <span class="hosted-banner__badge">✦ Quickstart</span>
+ <span class="hosted-banner__badge">✦ Recommended</span>📝 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.
| <span class="hosted-banner__badge">✦ Quickstart</span> | |
| <span class="hosted-banner__badge">✦ Recommended</span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.html`
at line 65, The badge text in the OwnConnections component template is
incorrect: in own-connections.component.html the span with class
"hosted-banner__badge" currently contains "Quickstart" but should read
"Recommended" per the PR requirement; update the inner text of that span
(hosted-banner__badge) to "Recommended" so the badge copy matches the spec.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit