Fix views staying hidden when shown during their hide animation - #6349
Open
jan-grzybek wants to merge 1 commit into
Open
jan-grzybek wants to merge 1 commit into
jan-grzybek wants to merge 1 commit into
Conversation
`UIView.setIsHidden(_:using:)` and `setIsHidden(_:withAnimationDuration:completion:)` only set `isHidden` once a hide animation completes. A show requested before then was compared against the stale `isHidden` and ignored, and the hide's completion then hid the view anyway. A show also never un-hid a view whose alpha was 0. In the media review screen, for example, turning "view once" on and off in quick succession leaves the caption field hidden, so no caption can be entered, and the Add Media button missing. Track the most recent animated change per view instead. It supersedes any pending change, including one whose animator hasn't started yet or doesn't finish at its end, and only its own animation and completion modify the view. Changes without animation apply immediately. Both variants share this logic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jan-grzybek
force-pushed
the
fix-media-caption-toolbar-button-visibility
branch
from
September 17, 2026 16:15
fa02200 to
4bd6ce1
Compare
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
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.
Contributor checklist
Administrative:
Commits and testing:
Description
UIView.setIsHidden(_:using:)andsetIsHidden(_:withAnimationDuration:completion:)(used bysetIsHidden(_:animated:completion:)) only setisHiddenonce a hide animation completes. A show requested before then is compared against the staleisHiddenand ignored, and the hide's completion then hides the view anyway.Steps to reproduce on
main(media review screen with a single photo):The same can happen to any view using these helpers when a show and a hide are requested in quick succession.
This change tracks the most recent animated change per view (as an associated object), shared by both variants:
Behavior for changes that don't overlap is unchanged, including calling
completionsynchronously when no animation is needed. It builds on the simplification in 1a77385. Both methods now have doc comments describing this behavior.Testing:
UIKitAnimationsTesttoSignalUITests: 14 tests covering both variants with real animators, waiting on completions rather than fixed delays. 8 of them fail onmain: a show during a hide (animated and not, for both variants), a show before the hide's animator has started, a show during a hide while the same animator also changes the view's alpha, showing a hidden view whose alpha is 0, and a hide without animation during a hide (which leaves alpha at 0). The other 6 pass onmainand guard against regressions, such as a repeated hide, or a hide whose first animator is reversed or never started.mainand verified they no longer reproduce with this change. Also checked that view once toggles normally, that editing the caption, the Done button and sending still work, and that the media viewer's controls hide and show on tap (setIsHidden(_:animated:)).SignalUITestspass on both simulators exceptRecipientPickerViewControllerTests.testPhoneNumberFinderParseResults, which fails the same way onmain(it depends on the device region).Scripts/precommit.pywith SwiftFormat 0.60.1 reports no changes.🤖 Generated with Claude Code