Revert "Improve desktop GUI UX and observation history workflows" - #56
Revert "Improve desktop GUI UX and observation history workflows"#56Dumdart wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85939a9820
ℹ️ 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 render_query(self, query: SnapshotQuery) -> None: | ||
| if query == self._rendered_query and self.has_unsaved_edits: | ||
| return | ||
| self._rendered_query = query | ||
| self._topic_filter.setText(query.topic_filter) |
There was a problem hiding this comment.
When the user is editing the snapshot filter, age, or limits while MQTT messages are arriving, _observe_messages() refreshes the snapshot, topics_changed invokes MainWindow._render_tree(), and that method calls render_query() with the last applied query. These unconditional assignments overwrite every unsaved field, so on an active broker the form can reset before the user clicks Apply or Reconnect & observe; retain the dirty-state guard instead of replacing an in-progress draft.
AGENTS.md reference: AGENTS.md:L30-L33
Useful? React with 👍 / 👎.
| def render(self) -> None: | ||
| context = ( | ||
| self._view_model.active_broker_profile.id, | ||
| self._view_model.topic if self._target_kind == "topic" else "", | ||
| ) | ||
| if self.has_unsaved_edits and self._draft_context == context: | ||
| return | ||
| if self._target_kind == "topic": |
There was a problem hiding this comment.
When editing an existing expectation on a broker receiving messages, the scheduled health evaluation emits health_changed, which calls this render() and reloads the selected persisted expectation via _load(). Because the dirty-draft check was removed, any name, condition, action, or description typed since selection is silently replaced roughly 750 ms after a live update; skip reloading when the current broker/topic context is unchanged and the form is dirty.
AGENTS.md reference: AGENTS.md:L30-L33
Useful? React with 👍 / 👎.
| if has_topics and not has_cached_values: | ||
| self._empty_state.setVisible(False) | ||
| return |
There was a problem hiding this comment.
After startup or reconnection, when the snapshot contains only persisted values, the caller passes both has_cached_values=True and has_topics=True. This condition does not hide the empty-state panel, and the connected/unfiltered path falls through to “No values observed yet” with a Reconnect & observe action even though the tree is populated with stored observations. Handle the cached-only case explicitly (or hide the empty state) so the UI does not contradict the displayed data.
Useful? React with 👍 / 👎.
| self.error.clear() | ||
| busy = self._view_model.is_busy("history-settings") | ||
| self.save.setEnabled(self._loaded and not busy) | ||
| self.save.setText("Applying…" if busy else "Apply history settings") | ||
| self.broker.setEnabled(not busy) | ||
| if self._view_model.history_settings_broker == self.broker.currentData(): | ||
| if self._view_model.history_settings_error: | ||
| self.error.setText(self._view_model.history_settings_error) | ||
| self.save.setEnabled(self._loaded) |
There was a problem hiding this comment.
While save_history_settings() is awaiting its worker-thread calls, this validation path leaves Apply enabled and no longer disables the broker selector. A second click consequently starts a task that fails with “operation is already in progress”; more seriously, switching to broker B during broker A's save can start a B load that is then invalidated by A's post-save load_history_settings(A), leaving the selector on B with no loaded settings until a manual reload. Consult the history-settings busy state and disable both controls for the duration of the operation.
AGENTS.md reference: AGENTS.md:L30-L33
Useful? React with 👍 / 👎.
| def topic_paths(self) -> list[str]: | ||
| subscriptions = self.subscriptions | ||
| observed_topics = tuple(item.topic for item in self._observer_snapshot.topics) | ||
| observed_topics = tuple(item.topic for item in self._snapshot.topics) | ||
| return list(collect_visible_topic_paths(subscriptions, observed_topics)) |
There was a problem hiding this comment.
When a subscription has more observed topics than the active snapshot result limit, or the snapshot topic filter excludes some matching topics, deriving observed_topics from _snapshot.topics removes those still-retained observations from the observer tree and from selected_wildcard_filter_summary. With the default limit this makes the 101st matching topic disappear and prevents users from selecting it even though its latest value remains available; build the navigation tree from an unfiltered observer snapshot and reserve the bounded snapshot for snapshot counts and inspection.
Useful? React with 👍 / 👎.
| def showEvent(self, event: QShowEvent) -> None: | ||
| super().showEvent(event) | ||
| if not self._initial_focus_set: | ||
| self._initial_focus_set = True | ||
| if self._inspector_stack.currentWidget() is self._topic_details: | ||
| self._topic_details.focus_payload() | ||
| else: | ||
| self._observer_tree.focus_search() | ||
| self._topic_details.focus_payload() |
There was a problem hiding this comment.
A QShowEvent is delivered again when the window is restored or shown after being hidden, not only during initial startup. Calling focus_payload() unconditionally therefore moves keyboard focus away from whichever control the user was using—such as topic search or an editor—and it also targets a hidden payload widget when the snapshot inspector is current. Limit initial-focus setup to the first show and choose a visible control for the active inspector.
Useful? React with 👍 / 👎.
1 participant
Reverts #55