Fix supplementary view + contained first responder reuse issue #507
Conversation
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
kyleve
changed the title
robmaceachern
approved these changes
Sep 20, 2023
There was a problem hiding this comment.
For practicality of implementation, we only track text fields right now. This could change, but is harder, given there's no generic "first responder changed" notification.
If we really need to, we can try out the UIWindowFirstResponderDidChangeNotification private api. I think scoping to text fields is totally fine though.
There was a problem hiding this comment.
of course it's from Peter Steinberger
I haven't, I couldn't repro in Flow layout which makes me think its either doing something similarly special, or I didn't quite poke things right. |
kyleve
added a commit
that referenced
this pull request
Nov 19, 2023
…rovements * origin/main: (123 commits) Update CHANGELOG.md (#508) Revert "Supplementary Tracking Fixes (#433)" Revert "Force layout before appear, to avoid animated updates (#505)" Force layout before appear, to avoid animated updates (#505) Update workaround versions (#506) Fix supplementary view + contained first responder reuse issue (#507) Supplementary Tracking Fixes (#433) Release 13.0.0 (#504) Update KeyboardObserver (#499) CONV-1435: Gravity layout frame change fix - Before: Layout gravity doesn't take into account frame changes. For example, when the orientation changes the scroll position (relative to the bottom) changes - After: Layout gravity takes frame changes into account so the when the frame changes the scroll position relative to the bottom remains unchanged Release 12.0.0 (#501) CONV-1435: Add scroll indicator insets to customScrollViewInsets (#500) CONV-1435: Gravity layout - Adds a new Chat App demo and a new behavior called verticalLayoutGravity. When verticalLayoutGravity is set to bottom, scrolling works the way you would expect for a messaging app. expose onKeyboardFrameWillChange on ListProperties onKeyboardFrameWillChange: Improve CHANGELOG, DocC CONV-1435: Custom keyboard adjustment mode - Adds a .custom KeyboardAdjustmentMode to fully customize inset behavior remove contentOffset from isContentScrollable calculation, improve comment Add ListView#isContentScrollable property - Add this property to ListView. It will be used in conjunction with upcoming so-called gravity scrolling changes to workaround an animation issue with paging Update CI script to reference the `xcodesorg/made/xcodes` package for installing simulator runtimes. (#494) Swipe Action Updates (#489) ...
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.
The below works around a (seeming?) bug or odd behavior in
UICollectionView, where it tries to be smart about recycling supplementary views that contain a first responder such as a text field. Specifically, it holds onto a supplementary view that contains a first responder, not immediately recycling it when it is scrolled out of view. That ensures that the keyboard isn't immediately dismissed, which would be jarring....Unfortunately, this doesn't seem to actually work in practice very well. When the supplementary view is scrolled back into view, and we're asked to dequeue a view, the collection view hands us back a different view, leading to double views that get stacked on top of each other in the layout, leading to a bunch of weirdness.
So, to work around this, we do a few things:
We begin tracking which supplementary views currently contain a first responder. For practicality of implementation, we only track text fields right now. This could change, but is harder, given there's no generic "first responder changed" notification. This code lives in
ListView.We update
ListLayoutContent.content(in: ...)to always return supplementary info when a supplementary view contains a first responder, even when out of frame. This ensures the supplementary view instance is kept alive by the collection view. You can see that happening here:collectionView(_:viewForSupplementaryElementOfKind:at:), we check to see if there's a live, existingvisibleContainer(aka the supplementary view) view, and if there is, we return that, instead of just dequeuing a new, wrong view.After all that, the correct thing happens.
Checklist
Please do the following before merging:
Mainsection.