Repository navigation
Colour Object Explorer and editor gutter - #91
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesConnection profile styling
Sequence Diagram(s)sequenceDiagram
participant SelectedConnection
participant QueryPanel
participant QueryEditor
participant QueryResultsTable
participant ObjectExplorerTree
SelectedConnection->>QueryPanel: provide active connection
QueryPanel->>QueryPanel: resolve color profile
QueryPanel->>QueryEditor: pass gutter colors
QueryPanel->>QueryResultsTable: pass active profile
SelectedConnection->>ObjectExplorerTree: provide selected node connection
ObjectExplorerTree->>ObjectExplorerTree: apply profile colors
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR extends connection color profiling to additional UI surfaces: the Object Explorer pane and the query editor’s line-number gutter, while keeping the editor canvas and other Monaco chrome unstyled. It also adds DOM-level test coverage for these profile-driven styles using a dark custom profile.
Changes:
- Apply the selected Object Explorer node’s connection profile to the explorer pane background and text.
- Apply the active query tab’s connection profile to the Monaco gutter (margin + line numbers) via CSS variables and scoped selectors.
- Add frontend DOM tests to verify gutter-only styling and Object Explorer background/foreground styling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/frontend/queryUi.test.tsx | Adds DOM coverage to assert query gutter CSS variables and Object Explorer inline background/foreground derived from the selected/active connection profile. |
| src/index.css | Adds scoped Monaco CSS overrides to color only the gutter margin and line numbers when profiling is enabled. |
| src/features/query/components/QueryPanel.tsx | Resolves the active connection’s color profile once and passes gutter colors to QueryEditor (and reuses the profile for results). |
| src/features/query/components/QueryEditor.tsx | Introduces CSS-variable-driven gutter styling by wrapping Monaco in a profiled container and exposing gutterBackground/gutterForeground props. |
| src/features/explorer/components/ObjectExplorerTree.tsx | Derives the selected node’s connection profile and applies its background/foreground (and text CSS vars) to the explorer pane container. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a29dc46ad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address reviewer comments: - Scope Object Explorer profile foreground to profiled surfaces
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/index.css:49
- In profiled mode this hover rule overrides TreeNode’s
hover:bg-*styling (and hover text-color changes are also overridden by the profiled foreground rule), resulting in no visible hover feedback for non-selected items. This is a usability/accessibility regression because users lose a key affordance for what they’re pointing at.
.object-explorer-profiled [role="treeitem"]:not([aria-selected="true"]):hover {
background-color: var(--object-explorer-background);
}
src/features/query/components/QueryEditor.tsx:129
- The
query-editor-profiledclass is currently toggled only bygutterBackground. If a caller ever provides onlygutterForeground(or if a future profile omits background), the gutter CSS won’t apply even though the CSS variables are set. Making the condition depend on either variable keeps this component’s API robust and self-consistent.
<div
data-testid="query-editor"
className={`h-full ${gutterBackground ? "query-editor-profiled" : ""}`}
style={editorStyle}
>
Address reviewer comments: - Restore profiled tree hover affordance - Activate gutter styling for either supplied colour
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 036c12d3c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address reviewer comments: - Preserve profile contrast during tree-row hover
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52b45fad36
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .object-explorer-profiled [role="treeitem"], | ||
| .object-explorer-profiled [role="treeitem"] button, | ||
| .object-explorer-profiled .object-explorer-profiled-text { | ||
| color: var(--object-explorer-foreground); |
There was a problem hiding this comment.
Keep the selected row on the validated background
When a custom profile is near the accepted 4.5:1 threshold, this selector applies its foreground to the selected tree item while TreeNode still overlays bg-accent/15 on that item's background. For example, the accepted #FFFFFF/#767676 pair becomes roughly #D9E8F3/#767676 on the selected row, only about 3.63:1, so selecting the node that activates the profile immediately defeats the validation guarantee. Preserve the profile background for selected rows or use a non-background selection indicator.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/features/explorer/components/ObjectExplorerTree.tsx:97
- "--object-explorer-background" is set on the explorer container, but no CSS consumes it (only "--object-explorer-foreground" is referenced). Setting unused custom properties makes future styling changes harder to reason about and can mislead readers into thinking it has an effect.
width: explorerWidth,
backgroundColor: selectedProfile?.background,
color: selectedProfile?.foreground,
"--object-explorer-background": selectedProfile?.background,
"--object-explorer-foreground": selectedProfile?.foreground,
src/features/explorer/components/ObjectExplorerTree.tsx:29
- The custom CSS variable "--object-explorer-background" is declared but not used anywhere in the codebase, which adds dead API surface and makes it harder to tell which styling hooks are actually supported. Either remove it, or add a concrete consumer of the variable.
This issue also appears on line 93 of the same file.
interface ObjectExplorerStyle extends CSSProperties {
"--object-explorer-background"?: string;
"--object-explorer-foreground"?: string;
}
Summary
Test plan
npm run test:frontendnpm run builddotnet test sidecar/Ssmsx.Sidecar.slnx --filter 'Category!=Integration'cargo test --manifest-path src-tauri/Cargo.tomlSummary by CodeRabbit
New Features
Bug Fixes
Tests