Fix #1275 layouts inconsistency in updateAnimated:completion of IGListBindingSectionController#1285
Conversation
…del but not invalidating their layout attributes for out-of-box ListBindingSectionController
Generated by 🚫 Danger |
…ndingSectionController
…/updates in IGListBindingSectionController
|
Hi, @rnystrom , I have added a experiments bitmask to enable the changes, please review again. |
|
@lorixx It seems that you are active maintainer now. Can you review this PR and import it? |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@candance has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Can this PR ready to be merged or any else problem exists? |
lorixx
left a comment
There was a problem hiding this comment.
Looks good overall, the only main feedback I have is around that IGTestDiffingSectionController is doing too much thing and should not be a one-for-all kind of class, would you mind creating a different SectionController and DataSource for the IGLayoutTestItem dependent use case?
|
@qhhonx has updated the pull request. Re-import the pull request |
…idateLayoutForUpdates test
03f1704 to
f775b3f
Compare
|
@qhhonx has updated the pull request. Re-import the pull request |
@lorixx I have accepted your suggestion and implemented it. Please review it again and give some comments if any. |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@lorixx has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
lorixx
left a comment
There was a problem hiding this comment.
Looks good to me, thanks Qinghua!
…istBindingSectionController (#1285) Summary: ## Changes in this pull request Issue fixed: #1275 This PR is straintforward solution as mentioned in #1275. When I finished it, I realized that it is not a problem in `IGListCollectionViewLayout Partial Optimization` and `IGListBindingSectionController` deserved it. Maybe `IGListCollectionViewLayout` is rarely used because of great builtin UICollectionViewFlowLayout or cells in section rarely need changing their sizes in practices. Despite it is not `IGListCollectionViewLayout`'s fault, I think it can be more optimized against `invalidateLayoutWithContext`. I would like to make an another PR to optimize it. Due to this PR just giving a proposed solution, it lacks of adding tests and changing log. When this solution is accepted, I would like to complete these todos. ### Checklist - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: #1285 Reviewed By: candance Differential Revision: D15592807 Pulled By: lorixx fbshipit-source-id: ae06abce896341509de4f3dfb73b3a7bc0a68c51
…istBindingSectionController (#1285) Summary: ## Changes in this pull request Issue fixed: #1275 This PR is straintforward solution as mentioned in #1275. When I finished it, I realized that it is not a problem in `IGListCollectionViewLayout Partial Optimization` and `IGListBindingSectionController` deserved it. Maybe `IGListCollectionViewLayout` is rarely used because of great builtin UICollectionViewFlowLayout or cells in section rarely need changing their sizes in practices. Despite it is not `IGListCollectionViewLayout`'s fault, I think it can be more optimized against `invalidateLayoutWithContext`. I would like to make an another PR to optimize it. Due to this PR just giving a proposed solution, it lacks of adding tests and changing log. When this solution is accepted, I would like to complete these todos. ### Checklist - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: #1285 Reviewed By: candance Differential Revision: D15592807 Pulled By: lorixx fbshipit-source-id: ae06abce896341509de4f3dfb73b3a7bc0a68c51
Changes in this pull request
Issue fixed: #1275
This PR is straintforward solution as mentioned in #1275. When I finished it, I realized that it is not a problem in
IGListCollectionViewLayout Partial OptimizationandIGListBindingSectionControllerdeserved it. MaybeIGListCollectionViewLayoutis rarely used because of great builtin UICollectionViewFlowLayout or cells in section rarely need changing their sizes in practices.Despite it is not
IGListCollectionViewLayout's fault, I think it can be more optimized againstinvalidateLayoutWithContext. I would like to make an another PR to optimize it.Due to this PR just giving a proposed solution, it lacks of adding tests and changing log. When this solution is accepted, I would like to complete these todos.
Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.