fix: refit images after canvas layout changes - #2634
Conversation
The stacked empty-state layout introduced in #2524 resizes its scroll area after the outer window resize event. Initial fitting therefore used a stale 640x480 area and left Fit to Window selected at 40.2%. Observe scroll-area resizing and recompute saved fit modes against the current image and available space. Preserve manual zoom and pan, with regressions for startup, dock resizing, and image navigation.
Record the corrected fit behavior for startup, layout changes, and image navigation in the release notes.
The introducing commit is not contained in any release tag. Omit the fragment for the startup regression fixed before the next release.
|
Verdict: request changes. Worth adding, but not ready to merge. Reviewed head
Worth adding and minimal: Yes. The production diff is small, reuses existing scale calculations, and observes the relevant widget through native Qt event handling. No new dependencies or speculative abstractions. Test quality: The added tests are meaningful integration tests, consistent with preferring integrated coverage over excessive unit testing. They exercise real application loading, Qt layout, window/dock resizing, and navigation, with geometric assertions rather than mocked calls. Seven of the nine added parameter cases fail against the base and all nine pass on the PR. However, changing the existing retained-brightness pan test to enter manual zoom hides the fit-mode pan regression. Retain coverage for panning while fitted, and cover Fit Width scroll restoration. Validation: 42 targeted tests passed on the PR; targeted Ruff formatting/lint, Gruff, and type checks passed. Regression comparisons used Qt's offscreen platform and real application handlers; native desktop QA and the exact historical 40.2% → 95.4% reproduction were not independently repeated. Removed the existing |
Pan and scroll do not switch the canvas to manual zoom. Restore their saved positions after recalculating fit scale, including when previous zoom is retained. Keep the fitted brightness-navigation regression and cover Fit Width scroll restoration and Fit Window pan restoration. Restore a release note scoped to dock resizing and navigation: these behaviors also fail on v7.4.1, unlike the unreleased startup regression.
|
Addressed both findings in f491c33.
Validation: 46 targeted tests passed, plus Ruff formatting/lint, gruff, type checking, and fragment formatting. The existing startup before/after screenshots remain valid; the additional navigation correction is covered by the regression tests and desktop check above. |
|
Re-review verdict: recommend merge after required CI passes. Reviewed
Validation: 46 targeted tests passed, plus targeted Ruff formatting/lint, Gruff, and type checks. Completed CI jobs pass; macOS test and artifact-install jobs remain queued at review time. This re-review used Qt's offscreen platform; native desktop QA was not repeated. |
Fixes the Fit to Window regression introduced by #2524 (
659c2e68): the urban-plaza image opened at 40.2% with fit selected, despite requiring 95.4% to fill the available canvas. The same reproduction passes on v7.4.1 and the introducing commit's parent.Fit modes now follow layout changes and recompute when navigating between images. Saved pan and scroll positions are restored in every zoom mode after the fitted scale is recalculated.
Validation: 46 targeted tests passed across zoom, empty-state, window-geometry persistence, and middle-drag scrolling; Ruff, gruff, and type checks passed. Desktop verification covered startup, window resizing, navigation, retained manual zoom, and Fit Width scroll restoration (240 → 240). Dock resizing is covered by automated Qt tests; native pointer automation could not drive the dock divider.
The changelog entry covers dock resizing and navigation behavior reproduced on released v7.4.1; the startup regression itself was unreleased.