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
WalkthroughAdds a browser preference to toggle a per-server color indicator in panel tabs and propagates server color metadata (bgcolor/fgcolor, server_id) from tree/tool modules through layout to TabTitle for conditional rendering. 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 browser.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
| const layoutDocker = React.useContext(LayoutDockerContext); | ||
| const internal = layoutDocker?.find(id)?.internal ?? defaultInternal; | ||
| const showServerColorIndicator = usePreferences( | ||
| React.useCallback((state) => state.getPreferencesForModule('browser')?.show_server_color_indicator ?? false, []) |
Contributor
There was a problem hiding this comment.
I think we can safely remove React.useCallback to simplify code.
| if (newTreeInfo?.server?.icon) { | ||
| const iconParts = newTreeInfo.server.icon.split(' '); | ||
| bgcolor = iconParts[1] || null; | ||
| fgcolor = iconParts[2] || null; |
Contributor
There was a problem hiding this comment.
what is this hack for?
| if (treeInfo?.server?.icon) { | ||
| const iconParts = treeInfo.server.icon.split(' '); | ||
| bgcolor = iconParts[1] || null; | ||
| fgcolor = iconParts[2] || null; |
| if (parentData?.server?.icon) { | ||
| const iconParts = parentData.server.icon.split(' '); | ||
| bgcolor = iconParts[1] || null; | ||
| fgcolor = iconParts[2] || null; |
| if (parentData?.server?.icon) { | ||
| const iconParts = parentData.server.icon.split(' '); | ||
| bgcolor = iconParts[1] || null; | ||
| fgcolor = iconParts[2] || null; |
| if (selectedNodeInfo?.server?.icon) { | ||
| const iconParts = selectedNodeInfo.server.icon.split(' '); | ||
| bgcolor = iconParts[1] || null; | ||
| fgcolor = iconParts[2] || null; |
Contributor
There was a problem hiding this comment.
i'm skeptical for this logic. kind of a hack,
| if (selectedNodeInfo?.server?.icon) { | ||
| const iconParts = selectedNodeInfo.server.icon.split(' '); | ||
| bgcolor = iconParts[1] || null; | ||
| fgcolor = iconParts[2] || null; |
| return ( | ||
| <Box display="flex" alignItems="center" title={attrs.tooltip} onContextMenu={onContextMenu} width="100%"> | ||
| {attrs.icon && <span className={`dock-tab-icon ${attrs.icon}`}></span>} | ||
| {showServerColorIndicator && attrs.bgcolor && !layoutDocker.isTabVisible(id) && ( |
Contributor
There was a problem hiding this comment.
how does this work, if a tab is open and server color is changed?
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
Comment on lines
+250
to
+253
| const parts = serverIcon.split(' '); | ||
| return { | ||
| bgcolor: parts[1] || null, | ||
| fgcolor: parts[2] || null, |
There was a problem hiding this comment.
Make server icon parsing resilient to spacing/class-token variations.
Line 250 assumes a strict single-space, single-class format. If spacing varies (or icon class contains multiple tokens), parts[1]/parts[2] can map to wrong values and break color rendering.
Proposed fix
export function getServerColors(serverIcon) {
- if (!serverIcon) {
+ if (!serverIcon || typeof serverIcon !== 'string') {
return { bgcolor: null, fgcolor: null };
}
- const parts = serverIcon.split(' ');
+ const parts = serverIcon.trim().split(/\s+/);
+ if (parts.length < 3) {
+ return { bgcolor: null, fgcolor: null };
+ }
return {
- bgcolor: parts[1] || null,
- fgcolor: parts[2] || null,
+ // Keep icon token(s) flexible; colors are the trailing 2 tokens.
+ bgcolor: parts[parts.length - 2] || null,
+ fgcolor: parts[parts.length - 1] || 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.
Suggested change
| const parts = serverIcon.split(' '); | |
| return { | |
| bgcolor: parts[1] || null, | |
| fgcolor: parts[2] || null, | |
| export function getServerColors(serverIcon) { | |
| if (!serverIcon || typeof serverIcon !== 'string') { | |
| return { bgcolor: null, fgcolor: null }; | |
| } | |
| const parts = serverIcon.trim().split(/\s+/); | |
| if (parts.length < 3) { | |
| return { bgcolor: null, fgcolor: null }; | |
| } | |
| return { | |
| // Keep icon token(s) flexible; colors are the trailing 2 tokens. | |
| bgcolor: parts[parts.length - 2] || null, | |
| fgcolor: parts[parts.length - 1] || null, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/pgadmin/static/js/utils.js` around lines 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).
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
| import ToolView, { getToolTabParams } from '../../ToolView'; | ||
| import { ApplicationStateProvider, useApplicationState } from '../../../../settings/static/ApplicationStateProvider'; | ||
| import { BROWSER_PANELS, WORKSPACES } from '../../../../browser/static/js/constants'; | ||
| import { getServerColors } from '../../utils'; |
There was a problem hiding this comment.
Remove unused import getServerColors.
The import is causing pipeline failures. This file receives bgcolor/fgcolor from tool modules via the event payload—it doesn't need to compute colors itself.
🐛 Proposed fix
-import { getServerColors } from '../../utils';📝 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.
Suggested change
| import { getServerColors } from '../../utils'; |
🧰 Tools
🪛 GitHub Actions: Check Javascript style
[error] 31-31: ESLint (unused-imports/no-unused-imports): 'getServerColors' is defined but never used.
🪛 GitHub Actions: Run feature tests on PostgreSQL
[error] 31-31: ESLint error: 'getServerColors' is defined but never used (unused-imports/no-unused-imports).
🪛 GitHub Actions: Run Javascript tests
[error] 31-10: ESLint error (unused-imports/no-unused-imports): 'getServerColors' is defined but never used.
🪛 GitHub Check: run-javascript-tests (macos-latest)
[failure] 31-31:
'getServerColors' is defined but never used
🪛 GitHub Check: run-javascript-tests (ubuntu-22.04)
[failure] 31-31:
'getServerColors' is defined but never used
🪛 GitHub Check: run-javascript-tests (windows-latest)
[failure] 31-31:
'getServerColors' is defined but never used
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/pgadmin/static/js/helpers/Layout/index.jsx` at 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.
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