#727: GTK: present confirmation dialogs through quadraui's native seam, not the in-canvas Dialog primitive - #738
Merged
JDonaghy merged 2 commits intoSep 1, 2026
Conversation
…message-dialog seam Bumps the quadraui pin to 973956e (quadraui#666: PlatformServices:: show_message_dialog + native_dialog_options), then wires GTK's dialog paint path to use it: render_content edge-triggers a native present (queued once per open, drained by tick() like PendingFileDialog, since the blocking call can't run from inside the paint callback) and suppresses the in-canvas draw while a native present is queued/in flight. The response maps back through the same "dialog:btn:N" id convention and Engine::dialog_click_button/dialog_cancel the in-canvas DialogHit::Button(id) mouse path already uses, so both paths produce identical EngineAction outcomes. Dialogs quadraui reports as not natively expressible (DialogTable, or a text-input field) keep rendering in-canvas exactly as before — no hand-maintained tag list, per native_dialog_options' own contract. TUI is untouched. Also fixes the quadraui#667 Minimap::layout deprecation warning the pin bump surfaced (layout_with_sizing with MinimapSizing::Fill, behaviour-identical to the old shim). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses review findings on the #727 branch: - Both new tests (native_dialog_presented_exactly_once_across_repeated_frames, text_input_dialog_stays_in_canvas_not_native) asserted only on internal bookkeeping (dialog_layout/native_dialog_shown/pending_native_dialog), which would stay green even if render_content's native branch also kept painting the in-canvas dialog underneath (the #587/#592 shape). Both now additionally assert on painted content via screen_contains: the native path confirms "Unsaved Changes" / "Save All & Quit" are absent from the Cairo surface across repeated frames, and the text-input path confirms the dialog's title and body prompt ARE painted. - Performed the RED/GREEN cycle CLAUDE.md's Testing section requires and is stating it here, since neither the original commit nor the test doc comments did: - native_dialog_presented_exactly_once_across_repeated_frames: dropping the `!self.native_dialog_shown.get()` guard makes the "must not re-queue" assertion fail (RED), confirmed, then restored (GREEN). Separately, forcing render_content's native branch to also call frame.draw(backend) underneath — the exact paint-bug shape the new screen_contains assertions exist to catch — fails the new "must not be painted" assertion (RED), confirmed, then restored (GREEN). This is the scenario the state-only assertions could not have caught. - text_input_dialog_stays_in_canvas_not_native: forcing the text-input dialog down the native branch (native_dialog_options short-circuited to always Some) fails the in-canvas assertions (RED), confirmed, then restored (GREEN). All fixed code is green; see the RED-verified doc comments on each test for the exact repro. - Non-blocking: reset native_dialog_shown eagerly in run_pending_native_dialog right after the blocking show_message_dialog call returns, instead of only lazily on the next render_content call that observes screen.dialog == None. Removes the latent trap where a hypothetical chained dialog opened synchronously from dialog_click_button/dialog_cancel would be silently suppressed by a stale native_dialog_shown == true. - Non-blocking: documented in DialogPanel::buttons' doc comment that dialogs with no explicit "cancel"-action button (e.g. the file-changed-on-disk Yes/No prompt) accept losing the native "Escape activates a specific labeled button" affordance, since GTK's own dismiss-without-choosing path still lands on dialog_cancel() either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #727
Automated PR opened by coordinator for review of issue #727.