Skip to content

Commit d5f4d85

Browse files
mzabuawalamzabuawala
committed
Feat: Added a option to colorize panel/tab header based on server color #2431 
1 parent d8a078a commit d5f4d85

File tree

13 files changed

+460
-15
lines changed

13 files changed

+460
-15
lines changed

‎.claude/settings.local.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"permissions": {
3+
"allow": [
4+
"Bash(git -C /Users/murtuza.zabuawala/Documents/projects/pgadmin4 log --oneline -20)",
5+
"Bash(ls:*)",
6+
"Bash(tree:*)"
7+
]
8+
}
9+
}

‎CHANGES_OVERVIEW.md‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
# Server Color Extraction Refactoring - Quick Overview
2+
3+
## What Was Done
4+
5+
### 1. ✅ Created Utility Function
6+
Added `getServerColors(serverIcon)` to `web/pgadmin/static/js/utils.js`
7+
8+
**Why it's needed**: The backend encodes server colors in the icon string like `"icon-pg #FF5733 #FFFFFF"`. Instead of parsing this in 6+ places, we now have one function with detailed documentation explaining this historical design decision.
9+
10+
**Example usage**:
11+
```javascript
12+
const { bgcolor, fgcolor } = getServerColors(serverInfo?.server?.icon);
13+
// Returns: { bgcolor: "#FF5733", fgcolor: "#FFFFFF" }
14+
```
15+
16+
### 2. ✅ Refactored 6 Modules
17+
Replaced all duplicated color extraction code with the utility function:
18+
19+
- ✅ SQLEditorModule.js
20+
- ✅ DebuggerModule.js (2 locations)
21+
- ✅ ERDModule.js
22+
- ✅ PsqlModule.js
23+
- ✅ SchemaDiffModule.js
24+
- ✅ node.js (browser tree)
25+
26+
**Before**:
27+
```javascript
28+
const iconParts = serverInfo.server.icon.split(' ');
29+
bgcolor = iconParts[1] || null;
30+
fgcolor = iconParts[2] || null;
31+
```
32+
33+
**After**:
34+
```javascript
35+
const { bgcolor, fgcolor } = getServerColors(serverInfo?.server?.icon);
36+
```
37+
38+
### 3. ✅ Added Server ID Tracking
39+
All tools now pass `server_id` when creating tabs:
40+
```javascript
41+
{
42+
title: panelTitle,
43+
icon: icon,
44+
bgcolor: bgcolor,
45+
fgcolor: fgcolor,
46+
server_id: serverInfo?.server?._id // NEW
47+
}
48+
```
49+
50+
### 4. ✅ Implemented Dynamic Color Updates
51+
Enhanced `TabTitle` component in `Layout/index.jsx` to:
52+
- Listen for `pgadmin:browser:node:updated` event
53+
- When a server's properties are updated, check if any open tabs belong to that server
54+
- Automatically update the color indicators without requiring tab refresh
55+
56+
**How it works**:
57+
1. User edits server properties and changes colors
58+
2. Server node fires `pgadmin:browser:node:updated` event
59+
3. All open tabs check if they belong to this server (via `server_id`)
60+
4. Matching tabs fetch fresh colors and update immediately
61+
62+
## Benefits
63+
64+
### Code Quality
65+
- ✅ Eliminated 6+ instances of duplicated code
66+
- ✅ Single source of truth for color extraction
67+
- ✅ Comprehensive documentation explaining the "why"
68+
69+
### Bug Fixes
70+
- ✅ **Fixed**: Server color changes now update all open tabs immediately
71+
- ✅ **Before**: Had to close and reopen tabs to see new colors
72+
- ✅ **After**: Colors update live when server properties change
73+
74+
### Maintainability
75+
- ✅ Easy to update parsing logic in one place
76+
- ✅ Clear documentation for future developers
77+
- ✅ Easier to refactor if backend changes
78+
79+
## Testing Checklist
80+
81+
### Basic Functionality
82+
- [ ] Open Query Tool - verify server color indicator appears
83+
- [ ] Open Debugger - verify server color indicator appears
84+
- [ ] Open ERD Tool - verify server color indicator appears
85+
- [ ] Open PSQL - verify server color indicator appears
86+
87+
### Dynamic Updates (The Fix!)
88+
- [ ] Open multiple tabs from same server
89+
- [ ] Edit server properties → change bgcolor/fgcolor
90+
- [ ] Save changes
91+
- [ ] **Verify**: All tabs from that server update colors immediately ✨
92+
- [ ] **Verify**: Tabs from other servers are unaffected
93+
94+
### Edge Cases
95+
- [ ] Server with no colors set (should handle gracefully)
96+
- [ ] Server with only bgcolor (no fgcolor)
97+
- [ ] Multiple servers with different colors
98+
99+
## Key Insight: Why This "Hack" Exists
100+
101+
The contributor asked "what is this hack for?" The answer:
102+
103+
**Historical Design**: The backend stores `bgcolor` and `fgcolor` as separate database columns, but concatenates them into the icon string for frontend consumption:
104+
105+
```python
106+
# Backend: web/pgadmin/browser/server_groups/servers/__init__.py
107+
def server_icon_and_background(is_connected, manager, server):
108+
server_background_color = ''
109+
if server and server.bgcolor:
110+
server_background_color = ' {0}'.format(server.bgcolor)
111+
if server.fgcolor:
112+
server_background_color = '{0} {1}'.format(
113+
server_background_color, server.fgcolor
114+
)
115+
return 'icon-{0}{1}'.format(manager.server_type, server_background_color)
116+
```
117+
118+
This creates strings like: `"icon-pg #FF5733 #FFFFFF"`
119+
120+
**Frontend must parse**: Since colors aren't sent as separate fields, the frontend must split the string and extract them.
121+
122+
**Our solution**: Instead of having this parsing logic scattered everywhere, we centralized it with clear documentation explaining why it exists.
123+
124+
## Notes for Reviewers
125+
126+
1. **No breaking changes**: All existing functionality preserved
127+
2. **Better maintainability**: One place to update if format changes
128+
3. **Bug fix included**: Dynamic color updates now work
129+
4. **Well documented**: Clear comments explain the historical context
130+
131+
## Future Considerations
132+
133+
The ideal solution would be to have the backend send colors as separate JSON fields:
134+
```json
135+
{
136+
"icon": "icon-pg",
137+
"bgcolor": "#FF5733",
138+
"fgcolor": "#FFFFFF"
139+
}
140+
```
141+
142+
But that would require backend API changes and is beyond the scope of this refactoring.

‎REFACTORING_SUMMARY.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Server Color Refactoring Summary
2+
3+
## Overview
4+
This refactoring addresses code duplication and a bug related to server color indicators in panel/tab headers. The changes implement a DRY approach and add dynamic color updates when server properties change.
5+
6+
## Problems Solved
7+
8+
### 1. Code Duplication ("The Hack")
9+
**Problem**: The same logic to extract bgcolor/fgcolor from server icon strings was duplicated across 6+ files:
10+
```javascript
11+
// Repeated everywhere:
12+
const iconParts = serverIcon.split(' ');
13+
bgcolor = iconParts[1] || null;
14+
fgcolor = iconParts[2] || null;
15+
```
16+
17+
**Root Cause**: The backend encodes structured data (icon CSS class, bgcolor, fgcolor) into a single space-separated string via `server_icon_and_background()` in Python. The format is: `"icon-class bgcolor fgcolor"` (e.g., `"icon-pg #FF5733 #FFFFFF"`).
18+
19+
While the Server model in the database has separate `bgcolor` and `fgcolor` fields, the backend concatenates them with the icon class for tree node representation, requiring frontend parsing.
20+
21+
**Solution**: Created a centralized utility function `getServerColors()` with comprehensive documentation explaining this design decision.
22+
23+
### 2. Server Color Update Bug
24+
**Problem**: When a user changes a server's bgcolor/fgcolor in server properties while tabs are open, those tabs don't update their color indicators until closed and reopened.
25+
26+
**Root Cause**: The TabTitle component only reads colors from cached `internal` data when `REFRESH_TITLE` event fires, never fetching fresh colors from the tree when server properties are updated.
27+
28+
**Solution**: Added an event listener for `pgadmin:browser:node:updated` in TabTitle component to dynamically update colors when server properties change.
29+
30+
## Changes Made
31+
32+
### 1. New Utility Function
33+
**File**: `web/pgadmin/static/js/utils.js`
34+
35+
Added `getServerColors(serverIcon)` function with:
36+
- Detailed JSDoc explaining the historical context
37+
- Clear example usage
38+
- Reference to the backend function responsible for this format
39+
- Safe handling of null/undefined inputs
40+
41+
### 2. Refactored Modules (6 files)
42+
All tool modules now use the centralized utility:
43+
44+
1. **SQLEditorModule.js**
45+
- Added `getServerColors` import
46+
- Replaced parsing logic with utility call
47+
- Added `server_id` to tab params
48+
49+
2. **DebuggerModule.js** (2 locations)
50+
- Direct debugger launch
51+
- Indirect debugger launch
52+
- Both now use utility and include `server_id`
53+
54+
3. **ERDModule.js**
55+
- Replaced parsing logic
56+
- Added `server_id` to tab params
57+
58+
4. **PsqlModule.js**
59+
- Replaced parsing logic
60+
- Added `server_id` to tab params
61+
62+
5. **SchemaDiffModule.js**
63+
- Replaced parsing logic
64+
- Added `server_id` to tab params
65+
66+
6. **node.js** (browser tree callbacks)
67+
- Replaced parsing logic in `change_server_background` callback
68+
69+
### 3. Dynamic Color Updates
70+
**File**: `web/pgadmin/static/js/helpers/Layout/index.jsx`
71+
72+
**Changes**:
73+
- Added imports for `pgAdmin` and `getServerColors`
74+
- Enhanced `TabTitle` component's useEffect to:
75+
- Listen for `pgadmin:browser:node:updated` event
76+
- Check if the updated node is a server matching this tab's server_id
77+
- Extract fresh colors from the updated server icon
78+
- Update both internal data and visual state
79+
- Properly clean up event listener on unmount
80+
81+
**How it works**:
82+
1. When user saves server properties, `pgadmin:browser:node:updated` event fires
83+
2. All open TabTitle components receive the event
84+
3. Each tab checks if `newNodeData._id` matches its stored `server_id`
85+
4. Matching tabs extract fresh colors and update their display
86+
5. Color indicators update immediately without tab refresh
87+
88+
### 4. Server ID Tracking
89+
All tool modules now pass `server_id` in their tab parameters:
90+
```javascript
91+
{
92+
title: panelTitle,
93+
icon: icon,
94+
bgcolor: bgcolor,
95+
fgcolor: fgcolor,
96+
server_id: serverId // NEW: enables color update matching
97+
}
98+
```
99+
100+
This enables the TabTitle component to identify which tabs belong to which server when properties change.
101+
102+
## Benefits
103+
104+
1. **DRY Principle**: Eliminated 6+ instances of duplicated parsing logic
105+
2. **Maintainability**: Single source of truth for color extraction
106+
3. **Documentation**: Clear explanation of the historical design decision
107+
4. **Bug Fix**: Server color changes now reflect immediately in all open tabs
108+
5. **Type Safety**: Consistent return type `{bgcolor: string|null, fgcolor: string|null}`
109+
6. **Future-Proof**: Easier to refactor if backend changes color encoding
110+
111+
## Testing Recommendations
112+
113+
1. **Color Extraction**:
114+
- Verify color indicators appear correctly when opening tools
115+
- Test with servers having no colors set (should handle gracefully)
116+
- Test with servers having only bgcolor (no fgcolor)
117+
118+
2. **Dynamic Updates**:
119+
- Open multiple tabs from same server
120+
- Change server's bgcolor/fgcolor in properties
121+
- Verify all tabs from that server update colors immediately
122+
- Verify tabs from other servers are unaffected
123+
124+
3. **Edge Cases**:
125+
- Open tabs, disconnect server, reconnect with different colors
126+
- Multiple tools open simultaneously (Query Tool, Debugger, ERD, etc.)
127+
- Tabs from different servers in same layout
128+
129+
## Future Improvements
130+
131+
1. **Backend Refactor**: Consider sending bgcolor/fgcolor as separate JSON fields instead of space-separated string
132+
2. **Performance**: Could debounce color updates if many tabs exist
133+
3. **Memory**: Consider weak references if tab count grows very large
134+
135+
## Files Modified
136+
137+
1. `web/pgadmin/static/js/utils.js` - New utility function
138+
2. `web/pgadmin/static/js/helpers/Layout/index.jsx` - Dynamic color updates
139+
3. `web/pgadmin/tools/sqleditor/static/js/SQLEditorModule.js`
140+
4. `web/pgadmin/tools/debugger/static/js/DebuggerModule.js`
141+
5. `web/pgadmin/tools/erd/static/js/ERDModule.js`
142+
6. `web/pgadmin/tools/psql/static/js/PsqlModule.js`
143+
7. `web/pgadmin/tools/schema_diff/static/js/SchemaDiffModule.js`
144+
8. `web/pgadmin/browser/static/js/node.js`
145+
146+
## Related Backend Code
147+
148+
- `web/pgadmin/browser/server_groups/servers/__init__.py` - `server_icon_and_background()` function
149+
- Database migration: `web/migrations/versions/02b9dccdcfcb_.py` - Added bgcolor/fgcolor columns

‎web/pgadmin/browser/register_browser_preferences.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,18 @@ def register_browser_preferences(self):
101101
)
102102
)
103103

104+
self.preference.register(
105+
'display', 'show_server_color_indicator',
106+
gettext("Show server color indicator in panel tabs?"), 'boolean',
107+
False,
108+
category_label=PREF_LABEL_DISPLAY,
109+
help_str=gettext(
110+
'If enabled, a colored circle indicator will be shown in panel '
111+
'tabs (Query Tool, ERD Tool, etc.) matching the server\'s custom '
112+
'background color.'
113+
)
114+
)
115+
104116
self.table_row_count_threshold = self.preference.register(
105117
'properties', 'table_row_count_threshold',
106118
gettext("Count rows if estimated less than"), 'integer', 2000,

‎web/pgadmin/browser/static/js/node.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,23 @@ define('pgadmin.browser.node', [
479479
'pgadmin:browser:node:' + _newNodeData._type + ':updated',
480480
_item, _newNodeData, _oldNodeData
481481
);
482+
483+
// Trigger a specific event for server color updates
484+
// This allows tabs to update their color indicators when server colors change
485+
if (_newNodeData._type === 'server') {
486+
// Extract colors from the icon string using getServerColors utility
487+
const newColors = commonUtils.getServerColors(_newNodeData?.icon);
488+
489+
pgBrowser.Events.trigger(
490+
'pgadmin:server:colors:updated',
491+
_newNodeData._id,
492+
{
493+
bgcolor: newColors.bgcolor,
494+
fgcolor: newColors.fgcolor,
495+
icon: _newNodeData.icon
496+
}
497+
);
498+
}
482499
},
483500
}
484501
);
@@ -708,9 +725,8 @@ define('pgadmin.browser.node', [
708725

709726
// Go further only if node type is a Server
710727
if (index !== -1) {
711-
// First element will be icon and second will be colour code
712-
let bgcolor = serverData.icon.split(' ')[1] || null,
713-
fgcolor = serverData.icon.split(' ')[2] || '';
728+
// Extract bgcolor and fgcolor from server icon
729+
const { bgcolor, fgcolor } = commonUtils.getServerColors(serverData.icon);
714730

715731
if (bgcolor) {
716732
let dynamic_class = 'pga_server_' + serverData._id + '_bgcolor';

‎web/pgadmin/static/js/ToolView.jsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export function getToolTabParams(panelId, toolUrl, formParams, tabParams, restor
4242
if(tabParams?.internal?.orig_title){
4343
tabParams.title = tabParams.internal.isDirty ? tabParams.internal.title.slice(0, -1): tabParams.internal.title;
4444
}
45+
4546
return {
4647
id: panelId,
4748
title: panelId,

0 commit comments

Comments
 (0)