Apply server foreground colour to object counts (#9766)#10041
Conversation
|
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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR extends the dynamic CSS in change_server_background to apply a server’s configured foreground color to object explorer children counts, and adds a v9.16 release notes entry documenting Issue Changesv9.16 Release Notes and Foreground Color Fix
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
dpage
force-pushed
the
fix-9766-fgcolor-counts
branch
2 times, most recently
from
434e6fa to
5692e06
Compare
The per-server dynamic style only set the foreground colour on span.file-name, so the collection "children count" (span.children-count) did not follow the configured foreground colour. Include span.children-count in the fgcolor rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dpage
force-pushed
the
fix-9766-fgcolor-counts
branch
from
5692e06 to
dd0bce1
Compare
There was a problem hiding this comment.
Pull request overview
Updates pgAdmin’s browser tree styling so per-server foreground colours apply consistently, improving readability of labels and adjacent metadata in the object explorer.
Changes:
- Extend the per-server foreground-colour CSS rule in
change_server_backgroundto also style collection object counts. - Add a v9.16 release note entry referencing Issue #9766.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/pgadmin/browser/static/js/node.js | Updates dynamic per-server CSS selectors so the configured foreground colour also applies to counts shown next to collection nodes. |
| docs/en_US/release_notes_9_16.rst | Documents the fix in the v9.16 bug fixes section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rg#9766) Issue pgadmin-org#9766 asked for both the object counts and the column type text to follow the server's custom foreground colour. The dynamic per-server CSS rule only recoloured the file-name and children-count spans, leaving the column type text (span.text-muted) at its default low-contrast colour. Recolour span.text-muted to a reduced-emphasis blend of the foreground colour using color-mix, so the datatype still reads as de-emphasised secondary text while following the server colour.
![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
🤖 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 `@web/pgadmin/browser/static/js/node.js`:
- Around line 748-750: The dynamic CSS rule .${dynamic_class} span.text-muted
uses color-mix() which isn’t supported by older browsers; add a fallback color
declaration before the color-mix line (e.g., a precomputed rgba/opacity value
derived from ${fgcolor}) and/or wrap color-mix in an `@supports` (selector:
color-mix) block so browsers fall back to the rgba; also verify that the
selector .${dynamic_class} span.text-muted actually targets the column type
labels referenced in Issue `#9766` and, if those labels use a different
element/class, update the selector to match the real DOM element.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e378c805-f533-486d-a76b-5b12b31addb6
📒 Files selected for processing (2)
docs/en_US/release_notes_9_16.rstweb/pgadmin/browser/static/js/node.js
✅ Files skipped from review due to trivial changes (1)
- docs/en_US/release_notes_9_16.rst
asheshv
left a comment
asheshv
left a comment
There was a problem hiding this comment.
LGTM, with one small follow-up. The selector and color-mix() usage match the existing convention in reactaspen.override.js.
One gap worth fixing in this PR or a follow-up: the new rule has no :hover / .pseudo-active variant for span.children-count, so on hover the count snaps to the theme textHoverFg while the label keeps the user's custom colour. Mirroring the :hover block already added for file-name would close that inconsistency.
Summary
Fixes #9766.
A server's custom foreground colour was applied to node labels (
span.file-name) but not to the object counts shown next to collection nodes, so the counts stayed the default colour and could be hard to read against the server background colour.Fix: include
span.children-countin the per-serverfgcolorCSS rule inchange_server_background.🤖 Generated with Claude Code
Summary by CodeRabbit