Feat: Added a option to colorize panel/tab header based on server color #2431#9799
Open
mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
Open
Feat: Added a option to colorize panel/tab header based on server color #2431#9799mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (10)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds a boolean browser preference to show a per-server color indicator in panel tabs, propagates server color metadata (bgcolor/fgcolor, server_id) from tree/tool modules through tool show events into the layout, and renders a conditional color dot in TabTitle based on preference and visibility. Changes
Sequence DiagramsequenceDiagram
actor User
participant Tool as Tool Module
participant Layout as LayoutDocker
participant TabTitle as TabTitle Component
participant Pref as Preference System
User->>Tool: open tool (SQLEditor/Debugger/...)
Tool->>Tool: getServerColors(server.icon)
Tool->>Layout: trigger('pgadmin:tool:show', {..., bgcolor, fgcolor, server_id})
Layout->>Layout: getPanel(..., bgcolor, fgcolor, server_id) -> store in internal
Layout->>TabTitle: provide internal props (including colors)
TabTitle->>Pref: read display.show_server_color_indicator
Pref-->>TabTitle: preference value
alt pref enabled & bgcolor present & tab hidden
TabTitle->>TabTitle: render circular color indicator
else
TabTitle->>TabTitle: do not render indicator
end
TabTitle-->>User: display tab title
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
mzabuawala
force-pushed
the
feat-query-tool-2431
branch
2 times, most recently
from
7e3e6df to
102aa02
Compare
![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
🧹 Nitpick comments (2)
web/pgadmin/tools/debugger/static/js/DebuggerModule.js (1)
382-389: Refactor duplicated color parsing into a local helper.The same
server.iconsplit/extract logic appears in both debugger launch paths; a shared helper in this module would reduce maintenance overhead.Also applies to: 540-547
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/tools/debugger/static/js/DebuggerModule.js` around lines 382 - 389, Extract the duplicated server.icon parsing into a small local helper (e.g., parseServerIcon(icon)) that accepts newTreeInfo.server.icon, splits it, and returns {bgcolor, fgcolor} (null when not present); replace the inline logic that sets bgcolor/fgcolor in both debugger launch paths (the snippet using newTreeInfo?.server?.icon around where bgcolor and fgcolor are assigned and the similar block at lines 540-547) to call this helper instead, leaving callers to destructure or assign the returned values.web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js (1)
226-233: Consider centralizing server-color parsing into a shared helper.
icon.split(' ')[1/2]is now repeated across multiple tool modules; extracting a shared utility would reduce drift and simplify future changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js` around lines 226 - 233, The repeated parsing of server icon colors (selectedNodeInfo.server.icon.split(' ')[1/2]) should be extracted into a shared helper to avoid duplication: create a utility function (e.g., parseServerIconColors(icon) -> {bgcolor, fgcolor}) in a common utilities module and export it, replace the inline code in SQLEditorModule.js (where bgcolor/fgcolor are set from selectedNodeInfo.server.icon) with a call to parseServerIconColors(selectedNodeInfo?.server?.icon) and update other tool modules to import and use the same helper so all modules derive bgcolor and fgcolor consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/pgadmin/browser/register_browser_preferences.py`:
- Line 106: The preference declaration gettext("Show server color indicator in
panel tabs?"), 'boolean', False exceeds the 79-character line length; fix by
wrapping the tuple across multiple lines or breaking the long string into
concatenated parts so the gettext(...) call or the tuple elements fit within the
79-char limit (e.g., place each tuple element on its own line inside the
surrounding list/tuple using parentheses) while keeping the symbol gettext("Show
server color indicator in panel tabs?"), 'boolean', False intact.
---
Nitpick comments:
In `@web/pgadmin/tools/debugger/static/js/DebuggerModule.js`:
- Around line 382-389: Extract the duplicated server.icon parsing into a small
local helper (e.g., parseServerIcon(icon)) that accepts newTreeInfo.server.icon,
splits it, and returns {bgcolor, fgcolor} (null when not present); replace the
inline logic that sets bgcolor/fgcolor in both debugger launch paths (the
snippet using newTreeInfo?.server?.icon around where bgcolor and fgcolor are
assigned and the similar block at lines 540-547) to call this helper instead,
leaving callers to destructure or assign the returned values.
In `@web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js`:
- Around line 226-233: The repeated parsing of server icon colors
(selectedNodeInfo.server.icon.split(' ')[1/2]) should be extracted into a shared
helper to avoid duplication: create a utility function (e.g.,
parseServerIconColors(icon) -> {bgcolor, fgcolor}) in a common utilities module
and export it, replace the inline code in SQLEditorModule.js (where
bgcolor/fgcolor are set from selectedNodeInfo.server.icon) with a call to
parseServerIconColors(selectedNodeInfo?.server?.icon) and update other tool
modules to import and use the same helper so all modules derive bgcolor and
fgcolor consistently.
🪄 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: 16c6121d-db9d-447c-9fe6-f6ae8f1935bd
📒 Files selected for processing (7)
web/pgadmin/browser/register_browser_preferences.pyweb/pgadmin/static/js/helpers/Layout/index.jsxweb/pgadmin/tools/debugger/static/js/DebuggerModule.jsweb/pgadmin/tools/erd/static/js/ERDModule.jsweb/pgadmin/tools/psql/static/js/PsqlModule.jsweb/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.jsweb/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
adityatoshniwal
requested changes
Mar 30, 2026
mzabuawala
force-pushed
the
feat-query-tool-2431
branch
2 times, most recently
from
3cfa44b to
8121fd0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/pgadmin/static/js/utils.js`:
- Around line 250-253: The current parsing of serverIcon is brittle because it
assumes single spaces and single-token icon classes; instead, split serverIcon
using whitespace (e.g., serverIcon.trim().split(/\s+/)) and extract the last one
or two tokens as bgcolor/fgcolor so multi-token icon classes don't shift indices
— update the logic around serverIcon splitting and the return that sets bgcolor
and fgcolor to use the last tokens (falling back to null if missing).
🪄 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: b963a62e-4e0f-4ebf-9625-5cc13e2b77f3
📒 Files selected for processing (9)
web/pgadmin/browser/register_browser_preferences.pyweb/pgadmin/browser/static/js/node.jsweb/pgadmin/static/js/helpers/Layout/index.jsxweb/pgadmin/static/js/utils.jsweb/pgadmin/tools/debugger/static/js/DebuggerModule.jsweb/pgadmin/tools/erd/static/js/ERDModule.jsweb/pgadmin/tools/psql/static/js/PsqlModule.jsweb/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.jsweb/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
✅ Files skipped from review due to trivial changes (2)
- web/pgadmin/tools/erd/static/js/ERDModule.js
- web/pgadmin/tools/debugger/static/js/DebuggerModule.js
🚧 Files skipped from review as they are similar to previous changes (5)
- web/pgadmin/browser/register_browser_preferences.py
- web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
- web/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.js
- web/pgadmin/tools/psql/static/js/PsqlModule.js
- web/pgadmin/static/js/helpers/Layout/index.jsx
mzabuawala
force-pushed
the
feat-query-tool-2431
branch
from
8121fd0 to
6cb541c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/pgadmin/static/js/helpers/Layout/index.jsx`:
- Line 31: Remove the unused import getServerColors from the top of
Layout/index.jsx; delete the line "import { getServerColors } from
'../../utils';" (and any leftover references if present) because bgcolor/fgcolor
are provided via event payloads and the module does not need to compute colors
itself, preventing the pipeline failure.
🪄 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: 54a14f6c-230a-4956-92a6-d3c21306475d
📒 Files selected for processing (10)
web/pgadmin/browser/register_browser_preferences.pyweb/pgadmin/browser/static/js/node.jsweb/pgadmin/static/js/ToolView.jsxweb/pgadmin/static/js/helpers/Layout/index.jsxweb/pgadmin/static/js/utils.jsweb/pgadmin/tools/debugger/static/js/DebuggerModule.jsweb/pgadmin/tools/erd/static/js/ERDModule.jsweb/pgadmin/tools/psql/static/js/PsqlModule.jsweb/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.jsweb/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
✅ Files skipped from review due to trivial changes (3)
- web/pgadmin/static/js/ToolView.jsx
- web/pgadmin/tools/debugger/static/js/DebuggerModule.js
- web/pgadmin/tools/erd/static/js/ERDModule.js
🚧 Files skipped from review as they are similar to previous changes (5)
- web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
- web/pgadmin/browser/register_browser_preferences.py
- web/pgadmin/static/js/utils.js
- web/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.js
- web/pgadmin/browser/static/js/node.js
mzabuawala
force-pushed
the
feat-query-tool-2431
branch
2 times, most recently
from
d5f4d85 to
5ce2809
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/pgadmin/static/js/helpers/Layout/index.jsx`:
- Around line 81-108: The handler serverColorsUpdatedHandler updates
panelData.internal and local attrs but never persists the layout, causing
restored tabs to revert; after calling setAttrs (inside
serverColorsUpdatedHandler) call layoutDocker's persistence API (e.g.,
layoutDocker.saveLayout() or the appropriate save/persist method on
layoutDocker) to persist the updated internal/bgcolor/fgcolor so the change
survives reloads, and keep the existing event deregistration logic as-is.
- Around line 46-47: The initial isVisible state is computed before
layoutDocker.layoutObj exists, so update the useEffect that registers the ACTIVE
listener to immediately set the correct visibility once the layout ref is
available by calling setIsVisible(layoutDocker?.isTabVisible(id) ?? false) at
the start of that effect; do the same fix in the other two effects that manage
visibility (the effects that use isVisible/setIsVisible and attach ACTIVE/tab
events) so they initialize visibility from layoutDocker?.isTabVisible(id) as
soon as the layout ref is assigned.
🪄 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: 5b8e48d8-0b27-4dba-806d-d62532203d24
📒 Files selected for processing (10)
web/pgadmin/browser/register_browser_preferences.pyweb/pgadmin/browser/static/js/node.jsweb/pgadmin/static/js/ToolView.jsxweb/pgadmin/static/js/helpers/Layout/index.jsxweb/pgadmin/static/js/utils.jsweb/pgadmin/tools/debugger/static/js/DebuggerModule.jsweb/pgadmin/tools/erd/static/js/ERDModule.jsweb/pgadmin/tools/psql/static/js/PsqlModule.jsweb/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.jsweb/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
✅ Files skipped from review due to trivial changes (4)
- web/pgadmin/static/js/ToolView.jsx
- web/pgadmin/tools/erd/static/js/ERDModule.js
- web/pgadmin/tools/debugger/static/js/DebuggerModule.js
- web/pgadmin/browser/static/js/node.js
🚧 Files skipped from review as they are similar to previous changes (4)
- web/pgadmin/tools/psql/static/js/PsqlModule.js
- web/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.js
- web/pgadmin/static/js/utils.js
- web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js
mzabuawala
force-pushed
the
feat-query-tool-2431
branch
from
5ce2809 to
8a456f2
Compare
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
2 participants
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 by CodeRabbit