Perform item reloads in a separate batch of updates #8
Conversation
This file contains 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
This works around a bug from ~iOS 14 that causes the last N items in a section to ignore any updates applied to them when N items are deleted from the same section and the items are at the end of the section. This also removes the conversion of inserts/removals in to updates, which reduces the chances of this code being needed and also more closely matches the intent of the user; if something has truly moved or been updated the appropriate methods can be used, while removals and inserts that happen to result in the same position will not animate as a refresh.
Similar changes made in the `fix-reload-delete-same-index-path` branch, but neither fully pass the new test.
Previously groups updates were only synthesised; there was no API to manually update a group
|
@MarsOpenNet @NewWay-opennet this is ready for review. If you'd prefer to not review this or know someone else that would be suitable to review this please let me know! I'm not aware of anyone that's particularly familiar with the internals of Composed, but this is quite specific to |
|
Hi @JosephDuffy , sorry for the late reply. I'll take a look for this. |
JosephDuffy
requested review from
ngptsporty
and removed request for
NewWay-opennet
ngptsporty
approved these changes
Mar 28, 2024
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.
This works around a bug from ~iOS 14 that causes the last N items in a section to ignore any updates applied to them when N items are deleted from the same section and the items are at the end of the section.
This also removes the conversion of inserts/removals in to updates, which reduces the need to account for these synthesised updates and also more closely matches the intent of the user; if something has truly moved or been updated the appropriate methods can be used, while removals and inserts that happen to result in the same position will not animate as a refresh.
This also fixes the crash that #6 was originally aiming to fix. It became much easier to reason about the changes once the reloads were not synthesised and were using the post-updates index paths. I think this probably fixes some other bugs. At a minimum the existing tests were updated or still passed, plus new tests have been added.
I also created opennetltd/ComposedUITests#1, which adds some more tests to the integration project. These are mostly used to validate that the unit tests here are correct according to
UICollectionView, e.g. it does not crash, request any unexpected cells, or fallback to areloadData.