From 0422205803c8f6980a447fdaa1dc270a486970cd Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 16:21:42 +0200 Subject: fix(rules): save the window size on Cancel and Save, not only on X The geometry was saved from closeEvent, and neither dialog button sends one: Cancel calls reject(), Save calls accept(), and only the window manager's X button produces a QCloseEvent. So the size and the column widths were kept for the one route out of three that a user almost never takes, and a resize followed by Cancel came back forgotten. The save moves to a done(int) override, which both buttons funnel through and which QWidget::close() also reaches. The test that covered this passed against the bug because it asserted with close(). It now drives all three routes rather than trusting one to stand for the others, and shows the dialog before the close leg: close() on a widget that was never visible returns early without reaching done(), so that assertion would otherwise prove nothing. Both traps recorded in CLAUDE.md, since neither is specific to this dialog. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'CLAUDE.md') diff --git a/CLAUDE.md b/CLAUDE.md index 67c0794..107fe28 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -317,6 +317,17 @@ current index invalid when nothing was current. A test that calls `selectAll()` view therefore passes against a missing selection guard, because no signal ever fires. Test multi-select from a row that is already current, which is also how a user reaches it. +**A `QDialog`'s buttons do not send a `QCloseEvent`.** `accept()` and `reject()` +go through `done(int)`, which hides the dialog without ever closing a window, so +a `closeEvent` override runs only for the window manager's X button. Anything a +dialog must persist on the way out belongs in a `done(int)` override, which both +buttons and `close()` reach. This shipped wrong in the rules dialog and the test +covering it passed, because the test used `close()` and the user used Cancel: +one route out of three. Assert every route. Underneath sits a second trap: +`close()` on a widget that was never shown returns early WITHOUT reaching +`done()`, so a test for the closed path has to `show()` the dialog first or it +asserts nothing at all. + **A queued load can outlive the state that started it.** `loadThread` crosses to the worker on a queued connection, so its reply lands after whatever the UI did in the meantime. The generation counter covers a superseded *query*, not a superseded *selection*: blanking the -- cgit v1.2.3