Harden reload against no-op orientation changes, modals, and lost windows - #46
Merged
Conversation
…t windows Follow-ups to the keep-measuring change, from a code review of Sources/ViewMonitor: - Skip the deferred orientation reload when the key window and its bounds are unchanged (faceUp/faceDown/upsideDown): the interface does not rotate there, and reloading wiped the selection, distance reference, and InfoView while the screen looked identical. Covered by a faceUp UI test that fails with the guard removed. - Coalesce deferred orientation reloads: one physical rotation emits several notifications, and each queued its own full teardown/re-scan. - End measuring when a transition comes from a modal presentation (alerts, sheets): keeping the shield would cover the dialog and make its buttons untappable. Detected via isBeingPresented in the swizzled hook; push transitions and rotation still keep measuring. Covered by lifecycle unit tests through a new transition seam. - Derive keep-measuring from launcherButton.isSelected instead of a stored flag. The flag could stay latched when a reload found no key window (armed shield with no visible UI re-appearing on a later transition) and could diverge from the button state in onToggle's guard path; derivation removes the state duplication entirely. - Extract beginMeasuring(with:on:) shared by onToggle and the restore path, removing the copy-pasted show sequence. - Pin the launcher to the top-right via autoresizing so that even a reload racing the window resize cannot strand it off screen. Co-Authored-By: Claude Fable 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.
Summary
Follow-ups to the 2.4.0 keep-measuring change, addressing findings from a code review of
Sources/ViewMonitor:orientationDidChangewithout rotating the interface; the deferred reload now skips when the key window and its bounds are unchanged, preserving the selection, distance reference, and InfoViewisBeingPresented, and modal transitions cancel measuring (push transitions and rotation still keep it)launcherButton.isSelectedinstead of a stored flag, removing state duplication that could stay latched when a reload found no key window (invisible armed shield re-appearing on a later transition)beginMeasuring(with:on:)extracted, deduplicating the show sequence between toggle-ON and the restore pathKnown limitations intentionally not addressed (niche / pre-existing): screens that fire
viewDidAppearrepeatedly on their own (e.g. auto-advancing page controllers) still re-scan per transition; iPad multi-scene key-window nondeterminism and transient key windows are pre-existingWindowProvidersemantics.Test plan
swiftlint --strict0 violations; both example apps build🤖 Generated with Claude Code