-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider supplementary views when sending display events #470
Conversation
@rnystrom updated the pull request - view changes |
Tells the handler that a supplementary view will be displayed in the IGListAdapter. | ||
|
||
@param view A supplementary view that will display. | ||
@param listAdapter The adapter managing the infra. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The adapter managing the infra."
what does that mean? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya these old docs suck (copy-pasted). Will use better words.
|
||
@param view A supplementary view that will display. | ||
@param listAdapter The adapter managing the infra. | ||
@param sectionController The section controller the view is in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe: The section controller that manages the view
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
@rnystrom updated the pull request - view changes |
Source/IGListAdapter.m
Outdated
@@ -17,7 +17,7 @@ | |||
#import "IGListSectionControllerInternal.h" | |||
|
|||
@implementation IGListAdapter { | |||
NSMapTable<UICollectionViewCell *, IGListSectionController<IGListSectionType> *> *_cellSectionControllerMap; | |||
NSMapTable<UICollectionReusableView *, IGListSectionController<IGListSectionType> *> *_cellSectionControllerMap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_reusableViewSectionControllerMap
since this now is not just cells?
|
||
if ([self.visibleListSections countForObject:sectionController] == 0) { | ||
[displayDelegate listAdapter:listAdapter willDisplaySectionController:sectionController]; | ||
[self.visibleCellObjectMap setObject:object forKey:view]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming again to reflect this is generic over UICollectionReusableView
and not just cells
Thanks @amonshiz! |
fd64cc3
to
49c364f
Compare
@rnystrom updated the pull request - view changes |
@rnystrom has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
This was a little bit of an invasive change with the display handler, but I think that this is the right call. When sending display events for objects, we should account for the supplementary view as part of the section controller. This is especially useful for headers and footers.
Note that this wont effect the working range API at all.
Fixes #300