connections: surface Edit schema as a text row on each card#1788
Open
karinakharchenko wants to merge 1 commit into
Open
connections: surface Edit schema as a text row on each card#1788karinakharchenko wants to merge 1 commit into
karinakharchenko wants to merge 1 commit into
Conversation
Move the per-connection "Edit schema" affordance out of the dark logo banner: it now sits as a third row below the banner as a full-width text button labeled "Edit schema" with an `auto_awesome` sparkle in the accented blue. A trailing `arrow_forward` icon fades in on hover. The text uses a subtly blue-tinted slate via --mdc-text-button-label-text-color so it doesn't compete with the banner at rest but lights up on hover. The card itself no longer highlights on any hover — the primary border / shadow are scoped via :has(.connectionLogoPreview:hover) so only the dark banner triggers the card-level highlight, leaving the schema row to react independently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Edit schema button in the connections list is redesigned from an icon-only Material button to a full-featured Material button with label and icons. The connection card container is refined with flex gap spacing and hover behavior triggered only when the logo preview is hovered, with matching dark-mode styling. ChangesSchema button redesign
🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the “Edit schema” affordance in the connections list card to a full-width text button and refines hover styling/spacing for the connection card.
Changes:
- Replaced the icon-only schema edit control with a text button containing leading/trailing icons.
- Updated card layout spacing and changed hover styling to trigger only when hovering the logo preview area.
- Added new button styling (including dark-mode tweaks and hover arrow animation).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/app/components/connections-list/own-connections/own-connections.component.html | Reworks the “Edit schema” control markup into a larger button with icons and label. |
| frontend/src/app/components/connections-list/own-connections/own-connections.component.css | Adjusts card spacing/hover behavior and adds styling for the new schema button presentation. |
Comments suppressed due to low confidence (1)
frontend/src/app/components/connections-list/own-connections/own-connections.component.css:1
- Using the relational selector
:has(...)can be problematic in production: it has historically had browser support gaps and may be disabled/unsupported in some embedded/enterprise environments; it can also be more expensive for style recalculation on large lists. If you need the parent highlight only when hovering the logo area, consider an alternative that doesn’t rely on:has, such as applying the hover visuals to.connectionLogoPreviewitself (or to an overlay element), or toggling a class on the parent via pointerenter/pointerleave in the component.
:host {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+35
to
44
| <a mat-button | ||
| class="connectionSchemaButton" | ||
| routerLink="/edit-database-schema/{{connectionItem.connection.id}}" | ||
| (click)="$event.stopPropagation()"> | ||
| <mat-icon class="connectionSchemaButton__ai">auto_awesome</mat-icon> | ||
| <span>Edit schema</span> | ||
| <mat-icon iconPositionEnd class="connectionSchemaButton__arrow">arrow_forward</mat-icon> | ||
| </a> | ||
| } | ||
| </a> |
Comment on lines
374
to
+383
| .connectionSchemaButton:hover { | ||
| color: #fff; | ||
| --mdc-text-button-label-text-color: var(--color-accentedPalette-500); | ||
| color: var(--color-accentedPalette-500) !important; | ||
| background-color: color-mix(in srgb, var(--color-accentedPalette-500), transparent 92%); | ||
| } | ||
|
|
||
| .connectionSchemaButton:hover .connectionSchemaButton__arrow { | ||
| opacity: 1; | ||
| transform: translate(0, -50%); | ||
| } |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/components/connections-list/own-connections/own-connections.component.html (1)
15-15:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRemove nested anchor: current markup is invalid and risks broken navigation/focus.
Line 35 introduces an
<a>inside the card<a>from Line 15. That’s invalid interactive nesting and can produce inconsistent browser behavior for click, keyboard, and routing.Suggested structure (split dashboard link and schema action into siblings)
- <li class="connectionItem"> - <a routerLink="/dashboard/{{connectionItem.connection.id}}" class="connection" - angulartics2On="click" - angularticsAction="Connections: open own db is clicked" - (click)="posthog.capture('Connections: open own db is clicked')"> + <li class="connectionItem"> + <div class="connection"> + <a routerLink="/dashboard/{{connectionItem.connection.id}}" + class="connectionDashboardLink" + angulartics2On="click" + angularticsAction="Connections: open own db is clicked" + (click)="posthog.capture('Connections: open own db is clicked')"> <div class="connectionLogoPreview"> ... </div> - `@if` (connectionItem.accessLevel === 'edit') { - <a mat-button - class="connectionSchemaButton" - routerLink="/edit-database-schema/{{connectionItem.connection.id}}" - (click)="$event.stopPropagation()"> - <mat-icon class="connectionSchemaButton__ai">auto_awesome</mat-icon> - <span>Edit schema</span> - <mat-icon iconPositionEnd class="connectionSchemaButton__arrow">arrow_forward</mat-icon> - </a> - } - </a> + </a> + `@if` (connectionItem.accessLevel === 'edit') { + <a mat-button + class="connectionSchemaButton" + routerLink="/edit-database-schema/{{connectionItem.connection.id}}"> + <mat-icon class="connectionSchemaButton__ai">auto_awesome</mat-icon> + <span>Edit schema</span> + <mat-icon iconPositionEnd class="connectionSchemaButton__arrow">arrow_forward</mat-icon> + </a> + } + </div> </li>Also applies to: 35-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/components/connections-list/own-connections/own-connections.component.html` at line 15, The markup nests an <a> for the dashboard link around the whole card and another <a> for the schema action (using connectionItem.connection.id), which is invalid; remove the nested anchor by keeping the outer routerLink="/dashboard/{{connectionItem.connection.id}}" on the card root and convert the inner schema action anchor into a non-anchor interactive element (e.g., a button or span with a click handler) or move the schema action out as a sibling element so both the dashboard routerLink and the schema action are separate, non-nested interactive controls (update any (click) handlers or routerLink usage accordingly in own-connections.component.html).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`:
- Line 344: The stylelint rule 'declaration-empty-line-before' is being violated
for the declaration "color: var(--mdc-text-button-label-text-color)
!important;"; open the CSS blocks containing that exact declaration (both
occurrences) and adjust the blank line immediately before the declaration to
match the project's stylelint setting (add a single empty line if rule requires
one, or remove the empty line if rule forbids it) so that the
declaration-empty-line-before rule is satisfied.
---
Outside diff comments:
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.html`:
- Line 15: The markup nests an <a> for the dashboard link around the whole card
and another <a> for the schema action (using connectionItem.connection.id),
which is invalid; remove the nested anchor by keeping the outer
routerLink="/dashboard/{{connectionItem.connection.id}}" on the card root and
convert the inner schema action anchor into a non-anchor interactive element
(e.g., a button or span with a click handler) or move the schema action out as a
sibling element so both the dashboard routerLink and the schema action are
separate, non-nested interactive controls (update any (click) handlers or
routerLink usage accordingly in own-connections.component.html).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ff9767c-6f48-4e59-b168-0916e7045482
📒 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
| align-self: stretch; | ||
| justify-content: center; | ||
| --mdc-text-button-label-text-color: color-mix(in srgb, var(--color-accentedPalette-500) 18%, #1f2937 82%); | ||
| color: var(--mdc-text-button-label-text-color) !important; |
There was a problem hiding this comment.
Fix stylelint declaration-empty-line-before violations.
Line 344 and Line 376 currently violate the configured stylelint rule and should be adjusted to keep lint clean.
Proposed lint-only formatting fix
.connectionSchemaButton {
position: relative;
align-self: stretch;
justify-content: center;
--mdc-text-button-label-text-color: color-mix(in srgb, var(--color-accentedPalette-500) 18%, `#1f2937` 82%);
+
color: var(--mdc-text-button-label-text-color) !important;
font-size: 13px;
@@
.connectionSchemaButton:hover {
--mdc-text-button-label-text-color: var(--color-accentedPalette-500);
+
color: var(--color-accentedPalette-500) !important;
background-color: color-mix(in srgb, var(--color-accentedPalette-500), transparent 92%);
}Also applies to: 376-376
🧰 Tools
🪛 Stylelint (17.11.1)
[error] 344-344: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/src/app/components/connections-list/own-connections/own-connections.component.css`
at line 344, The stylelint rule 'declaration-empty-line-before' is being
violated for the declaration "color: var(--mdc-text-button-label-text-color)
!important;"; open the CSS blocks containing that exact declaration (both
occurrences) and adjust the blank line immediately before the declaration to
match the project's stylelint setting (add a single empty line if rule requires
one, or remove the empty line if rule forbids it) so that the
declaration-empty-line-before rule is satisfied.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
auto_awesomesparkle and the label "Edit schema".arrow_forwardicon fades in on hover (positioned absolutely so the label stays centered when the arrow appears/disappears).--mdc-text-button-label-text-colorso it's calm at rest and lights up to the accent blue on hover — the sparkle is always blue.:has(.connectionLogoPreview:hover), so only the dark banner triggers it. Hovering the schema row no longer pulses the whole card.Test plan
/edit-database-schema/<id>(does not also open the dashboard)editaccess do not show the row🤖 Generated with Claude Code
Summary by CodeRabbit