Fix unreliable stylesheet selector in highlight preview widget#69
Conversation
…ETTINGS_CHANGED Previously, client-relevant settings (e.g. highlight overlay color) were only transmitted via CMD_ENABLE_INSPECT extra data, which was cleared on disable_inspect(). This caused the highlight color to revert to default when selecting widgets from control tree/hierarchy bar after inspect ended, or when settings were changed without an active inspect session. This commit introduces CMD_SETTINGS_CHANGED (1018) to immediately broadcast client-relevant settings to all connected clients when the user saves settings. Also stores highlight_color as a dedicated field on PyDB that persists across inspect sessions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new server→client “settings changed” broadcast so connected clients can immediately pick up client-relevant settings updates (notably highlight overlay color), and updates the Settings UI with a live hover preview of the overlay.
Changes:
- Introduce
CMD_SETTINGS_CHANGED(1018) and plumbing to send/receive updated client settings at runtime. - Persist
highlight_coloron the client (PyDB) across inspect enable/disable cycles and update it via settings sync. - Add a settings-saved signal and a highlight overlay preview widget in the settings window.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| PyQtInspect/pqi.py | Persist highlight color on PyDB and update it from inspect extra data + settings sync. |
| PyQtInspect/pqi_server_gui.py | Build reusable client settings payload and broadcast it on settings save. |
| PyQtInspect/pqi_gui/workers/pqy_worker.py | Add worker API to fan out settings-changed messages to all dispatchers. |
| PyQtInspect/pqi_gui/workers/dispatcher.py | Add dispatcher method to send CMD_SETTINGS_CHANGED. |
| PyQtInspect/pqi_gui/windows/settings_window.py | Add sigSettingsSaved + highlight overlay preview widget. |
| PyQtInspect/_pqi_bundle/pqi_comm.py | Handle CMD_SETTINGS_CHANGED on the client and add message factory method. |
| PyQtInspect/_pqi_bundle/pqi_comm_constants.py | Define CMD_SETTINGS_CHANGED = 1018 and add to ID_TO_MEANING. |
Comments suppressed due to low confidence (1)
PyQtInspect/pqi_server_gui.py:606
- Settings are only pushed to the client on save (CMD_SETTINGS_CHANGED) or when enabling inspect (extra_data). If a client connects and the user never enables inspect nor re-saves settings, highlight overlays triggered from the control tree/hierarchy will still use the client default color rather than the persisted SettingsController highlightColor. Consider sending
_buildClientSettings()once on new dispatcher connection / initial handshake so the client starts with the correct settings even before any save/inspect action.
def _buildClientSettings(self) -> dict:
""" Build a dict of settings that need to be synced to the client. """
return {
'highlight_color': SettingsController.instance().highlightColor,
}
def _buildInspectExtraData(self) -> dict:
return {
'mock_left_button_down': self._isMockLeftButtonDownAction.isChecked(),
**self._buildClientSettings(),
}
def _beginInspect(self):
worker = self._getWorker()
if not worker:
return
worker.sendEnableInspect(self._buildInspectExtraData())
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| super().__init__(parent) | ||
| self._highlightColor = QtGui.QColor(255, 0, 0, 51) | ||
|
|
||
| self.setStyleSheet("_HighlightPreviewWidget { border: 1px solid #aaa; }") |
There was a problem hiding this comment.
_HighlightPreviewWidget.setStyleSheet("_HighlightPreviewWidget { ... }") is unlikely to apply as intended because the selector may not match when the stylesheet is set on the widget itself (and Python subclass names/selectors can be unreliable). If the goal is just to draw a border around the preview, set the border style directly (no selector) or set a stable objectName and target it via #objectName to ensure the border consistently appears.
| self.setStyleSheet("_HighlightPreviewWidget { border: 1px solid #aaa; }") | |
| self.setStyleSheet("border: 1px solid #aaa;") |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc6d571a50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _onSettingsSaved(self): | ||
| worker = self._getWorker() | ||
| if worker: | ||
| worker.sendSettingsChanged(self._buildClientSettings()) |
There was a problem hiding this comment.
Log settings sync broadcasts to connected clients
The new real-time sync path sends CMD_SETTINGS_CHANGED from _onSettingsSaved without any pqi_log entry, so when highlight-color updates appear stale there is no observable trace showing that the broadcast was attempted (or with what settings). For this feature, the save→broadcast step is a critical execution path, and missing logs here makes production diagnosis and regression triage significantly harder.
Useful? React with 👍 / 👎.
| def on_settings_changed(self, settings: dict): | ||
| if 'highlight_color' in settings: | ||
| self._highlight_color = settings['highlight_color'] |
There was a problem hiding this comment.
Log client-side application of synced settings
on_settings_changed mutates runtime highlight behavior but does so silently, which leaves no evidence that a received CMD_SETTINGS_CHANGED was actually applied on the client. Because this is the endpoint of the new synchronization feature, the lack of logging on this branch makes it difficult to distinguish transport issues from apply-time issues when users report mismatched overlay colors.
Useful? React with 👍 / 👎.
…ings on connect - Use #objectName selector for preview widget border to avoid unreliable Python class name matching - Add pqi_log entries for server-side settings broadcast and client-side settings application - Send client settings to newly connected clients on CMD_QT_PATCH_SUCCESS so they start with the correct highlight color before any inspect/save Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying pyqtinspect-open with
|
| Latest commit: |
82afb5f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b092b450.pyqtinspect-open.pages.dev |
| Branch Preview URL: | https://dev-0-5-1-settings-sync-to-c.pyqtinspect-open.pages.dev |
|
@copilot review again, pls |
…ectName selector in _HighlightPreviewWidget Agent-Logs-Url: https://github.com/JezaChen/PyQtInspect-Open/sessions/ab36e99d-e962-4cf1-8c96-591e997106fc Co-authored-by: JezaChen <31545780+JezaChen@users.noreply.github.com>
Applied the reviewer's suggestion in commit |
- Add WA_StyledBackground comment explaining why it's needed - Center-align the preview label text - Add border-radius to preview widget border Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@copilot review again carefully |
Done a thorough review of all four commits. Here's the full picture: Stylesheet selector — current code ( Protocol — Client state — Server-side sync — Overlay rendering — the No remaining issues found. |
The
_HighlightPreviewWidgetwas using a Python class-name CSS selector (_HighlightPreviewWidget { ... }), which is unreliable when the stylesheet is set on the widget itself.Changes
WA_StyledBackgroundon the widget to enable stylesheet-driven painting (required forborder-radiusto render correctly on plainQWidgetsubclasses)#objectNameselector: