Feat: Added a option to colorize panel/tab header based on server color #2431#9799
Feat: Added a option to colorize panel/tab header based on server color #2431#9799mzabuawala wants to merge 1 commit intopgadmin-org:masterfrom
Conversation
WalkthroughThis PR introduces a new browser preference to control the visibility of server color indicators in tool panel tabs. Implementation changes across tool modules (Query Editor, Debugger, ERD, PSQL, Schema Diff) extract color values from server icons and propagate them through the layout system for conditional rendering. Changes
Sequence DiagramsequenceDiagram
actor User
participant Tool as Tool Module<br/>(SQLEditor/Debugger/etc)
participant Layout as Layout System<br/>(LayoutDocker)
participant TabTitle as TabTitle<br/>Component
participant Pref as Preference<br/>System
User->>Tool: Trigger tool panel
Tool->>Tool: Extract bgcolor/fgcolor<br/>from server.icon
Tool->>Layout: trigger('pgadmin:tool:show')<br/>with bgcolor/fgcolor
Layout->>Layout: getPanel()<br/>store colors in internal
Layout->>TabTitle: Pass internal colors<br/>to TabTitle props
TabTitle->>Pref: usePreferences()<br/>read show_server_color_indicator
Pref-->>TabTitle: preference value
alt show_server_color_indicator enabled<br/>& bgcolor present & tab hidden
TabTitle->>TabTitle: Render circular<br/>color indicator
else
TabTitle->>TabTitle: Skip indicator
end
TabTitle-->>User: Display tab with<br/>optional color indicator
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
4dc48d6 to
7e3e6df
Compare
7e3e6df to
102aa02
Compare
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
|
|
||
| self.preference.register( | ||
| 'display', 'show_server_color_indicator', | ||
| gettext("Show server color indicator in panel tabs?"), 'boolean', False, |
There was a problem hiding this comment.
Fix pycodestyle E501 on Line 106.
The new preference line exceeds the 79-char limit and is currently failing CI.
✂️ Suggested style-only fix
- gettext("Show server color indicator in panel tabs?"), 'boolean', False,
+ gettext("Show server color indicator in panel tabs?"),
+ 'boolean', False,📝 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.
| gettext("Show server color indicator in panel tabs?"), 'boolean', False, | |
| gettext("Show server color indicator in panel tabs?"), | |
| 'boolean', False, |
🧰 Tools
🪛 GitHub Actions: Check Python style
[error] 106-106: pycodestyle reported E501: line too long (80 > 79 characters).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/pgadmin/browser/register_browser_preferences.py` at 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.
Summary by CodeRabbit