Skip to content
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

[ASCollectionView] Add delegate bridging and index space translation for missing UICollectionViewLayout properties. #440

Merged
merged 3 commits into from
Jul 17, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## master

* Add your own contributions to the next release on the line below this with your name.
- [ASCollectionView] Add delegate bridging and index space translation for missing UICollectionViewLayout properties. [Scott Goodson](https://github.com/appleguy)
- [ASTextNode2] Add initial implementation for link handling. [Scott Goodson](https://github.com/appleguy) [#396](https://github.com/TextureGroup/Texture/pull/396)
- [ASTextNode2] Provide compile flag to globally enable new implementation of ASTextNode: ASTEXTNODE_EXPERIMENT_GLOBAL_ENABLE. [Scott Goodson](https://github.com/appleguy) [#396](https://github.com/TextureGroup/Texture/pull/410)

Expand Down
112 changes: 97 additions & 15 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -622,10 +622,16 @@ - (CGSize)sizeForElement:(ASCollectionElement *)element
return CGSizeZero;
}

// Get the indexPath from the pendingMap, as we're about to query the delegate (and it will reference its current data source).
// Often this is a translation compared to the visibleMap where the element came from (UICollectionViewLayout requesting sizeForItem).
NSIndexPath *indexPath = [_dataController.pendingMap indexPathForElement:element];
NSString *supplementaryKind = element.supplementaryElementKind;
NSIndexPath *indexPath = [_dataController.visibleMap indexPathForElement:element];
ASSizeRange sizeRange;
if (supplementaryKind == nil) {
if (indexPath == nil) {
// In this case, the latest data no longer has the element, so we can't ask the delegate for its latest constrained size.
// The element is still visible, but will be deleted soon. We can reuse the last-known constrainedSize for the element.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The element is still visible

"visible" is quite misleading here, and so does the "vibisbleMap" name. We should change it to "current" or something similar that indicates that the element is still recognized by UIKit objects.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I agree that if we clarified the terminology here, and used it consistently everywhere (and we could even start putting it in method names, or comments at the start of methods), it would be easier to ensure correctness.

Ideas for "UIKit" space:

  • View
  • UIKit
  • Committed

Ideas for "Latest" space:

  • Source
  • App
  • DataSource
  • Latest

I feel like there must be a better, more parallel set of names. One trouble with "current" is that you could imagine that being the current UIKit state, or the current App state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UIDataSourceTranslating uses "Presentation." I kind of like that name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"DataSource" and "Presentation", or "Processed" and "Presenting"?

sizeRange = element.constrainedSize;
} else if (supplementaryKind == nil) {
sizeRange = [self dataController:_dataController constrainedSizeForNodeAtIndexPath:indexPath];
} else {
sizeRange = [self dataController:_dataController constrainedSizeForSupplementaryNodeOfKind:supplementaryKind atIndexPath:indexPath];
Expand Down Expand Up @@ -949,7 +955,8 @@ - (NSInteger)collectionView:(UICollectionView *)collectionView numberOfItemsInSe
return [_dataController.visibleMap numberOfItemsInSection:section];
}

- (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollectionViewLayout *)collectionViewLayout sizeForItemAtIndexPath:(NSIndexPath *)indexPath
- (CGSize)collectionView:(UICollectionView *)collectionView
layout:(UICollectionViewLayout *)layout sizeForItemAtIndexPath:(NSIndexPath *)indexPath
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you changed the parameter name from collectionViewLayout to layout, you need to update this line.

{
ASDisplayNodeAssertMainThread();
ASCollectionElement *element = [_dataController.visibleMap elementForItemAtIndexPath:indexPath];
Expand All @@ -961,7 +968,9 @@ - (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollection
ASCellNode *cell = element.node;
if (cell.shouldUseUIKitCell) {
if ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:sizeForItemAtIndexPath:)]) {
CGSize size = [(id)_asyncDelegate collectionView:collectionView layout:collectionViewLayout sizeForItemAtIndexPath:indexPath];
CGSize size = [(id)_asyncDelegate collectionView:collectionView
layout:layout
sizeForItemAtIndexPath:indexPath];
cell.style.preferredSize = size;
return size;
}
Expand All @@ -970,45 +979,118 @@ - (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollection
return [self sizeForElement:element];
}

- (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollectionViewLayout *)layout referenceSizeForHeaderInSection:(NSInteger)section
- (NSIndexPath *)pendingIndexPathForSection:(NSInteger)section
{
// NOTE: For now, we don't translate length = 1 NSIndexPaths between spaces. Use the 0th item until then.
NSIndexPath *sectionIndexPath = [NSIndexPath indexPathForItem:0 inSection:section];
ASCollectionElement *element = [_dataController.visibleMap elementForItemAtIndexPath:sectionIndexPath];
NSIndexPath *pendingIndexPath = element ? [_dataController.pendingMap indexPathForElement:element] : nil;
return pendingIndexPath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be - (NSInteger)dataSourceSectionForSection:(NSInteger)section? I could see not-checking-NSNotFound on the caller side being a problem. Not checking nil is roughly as likely to occur but slightly less likely to cause issues immediately.

Currently ASSection objects are only created if the user implements the optional contextForSection: API. We should instead create ASSections unconditionally and use them as identifiers, similar to ASCollectionElement. That way looking up the indexes of sections between maps is easy!!


}

#define ASFlowLayoutDefault(layout, property, default) \
({ \
UICollectionViewFlowLayout *flowLayout = ASDynamicCast(layout, UICollectionViewFlowLayout); \
flowLayout ? flowLayout.property : default; \
})

- (UIEdgeInsets)collectionView:(UICollectionView *)collectionView
layout:(UICollectionViewLayout *)layout insetForSectionAtIndex:(NSInteger)section
{
if ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:insetForSectionAtIndex:)]) {
NSIndexPath *appIndexPath = [self pendingIndexPathForSection:section];
// If the section no longer exists in the latest dataSource state, fall through to the defaults below.
if (appIndexPath) {
return [(id)_asyncDelegate collectionView:collectionView
layout:layout insetForSectionAtIndex:appIndexPath.section];
}
}
return ASFlowLayoutDefault(layout, sectionInset, UIEdgeInsetsZero);
}

- (CGFloat)collectionView:(UICollectionView *)collectionView
layout:(UICollectionViewLayout *)layout minimumInteritemSpacingForSectionAtIndex:(NSInteger)section
{
if ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:minimumInteritemSpacingForSectionAtIndex:)]) {
NSIndexPath *appIndexPath = [self pendingIndexPathForSection:section];
// If the section no longer exists in the latest dataSource state, fall through to the defaults below.
if (appIndexPath) {
return [(id)_asyncDelegate collectionView:collectionView
layout:layout minimumInteritemSpacingForSectionAtIndex:appIndexPath.section];
}
}
return ASFlowLayoutDefault(layout, minimumInteritemSpacing, 10.0); // UIKit documents this default to be 10.0.
}

- (CGFloat)collectionView:(UICollectionView *)collectionView
layout:(UICollectionViewLayout *)layout minimumLineSpacingForSectionAtIndex:(NSInteger)section
{
if ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:minimumLineSpacingForSectionAtIndex:)]) {
NSIndexPath *appIndexPath = [self pendingIndexPathForSection:section];
// If the section no longer exists in the latest dataSource state, fall through to the defaults below.
if (appIndexPath) {
return [(id)_asyncDelegate collectionView:collectionView
layout:layout minimumLineSpacingForSectionAtIndex:appIndexPath.section];
}
}
return ASFlowLayoutDefault(layout, minimumLineSpacing, 10.0); // UIKit documents this default to be 10.0.
}

- (CGSize)collectionView:(UICollectionView *)collectionView
layout:(UICollectionViewLayout *)layout referenceSizeForHeaderInSection:(NSInteger)section
{
ASDisplayNodeAssertMainThread();
NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:section];
ASCollectionElement *element = [_dataController.visibleMap supplementaryElementOfKind:UICollectionElementKindSectionHeader
atIndexPath:indexPath];
if (element == nil) {
return CGSizeZero;
return ASFlowLayoutDefault(layout, headerReferenceSize, CGSizeZero); // UIKit documents this default to be CGSizeZero.
}

if (element.node.shouldUseUIKitCell && _asyncDelegateFlags.interop) {
if ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:referenceSizeForHeaderInSection:)]) {
return [(id)_asyncDelegate collectionView:collectionView layout:layout referenceSizeForHeaderInSection:section];
NSIndexPath *appIndexPath = [self pendingIndexPathForSection:section];
// If the section no longer exists in the latest dataSource state, fall through to the defaults below.
if (appIndexPath) {
return [(id)_asyncDelegate collectionView:collectionView layout:layout referenceSizeForHeaderInSection:appIndexPath.section];
}
}
return ASFlowLayoutDefault(layout, headerReferenceSize, CGSizeZero); // UIKit documents this default to be CGSizeZero.
} else {
// In this case, we have an ASCellNode-based Supplementary Node as a header. Ask it what size it wants to be.
return [self sizeForElement:element];
}

return [self sizeForElement:element];
}

- (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollectionViewLayout *)layout referenceSizeForFooterInSection:(NSInteger)section
- (CGSize)collectionView:(UICollectionView *)collectionView
layout:(UICollectionViewLayout *)layout referenceSizeForFooterInSection:(NSInteger)section
{
ASDisplayNodeAssertMainThread();
NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:section];
ASCollectionElement *element = [_dataController.visibleMap supplementaryElementOfKind:UICollectionElementKindSectionFooter
atIndexPath:indexPath];
if (element == nil) {
return CGSizeZero;
return ASFlowLayoutDefault(layout, footerReferenceSize, CGSizeZero); // UIKit documents this default to be CGSizeZero.
}

if (element.node.shouldUseUIKitCell && _asyncDelegateFlags.interop) {
if ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:referenceSizeForFooterInSection:)]) {
return [(id)_asyncDelegate collectionView:collectionView layout:layout referenceSizeForFooterInSection:section];
NSIndexPath *appIndexPath = [self pendingIndexPathForSection:section];
// If the section no longer exists in the latest dataSource state, fall through to the defaults below.
if (appIndexPath) {
return [(id)_asyncDelegate collectionView:collectionView layout:layout referenceSizeForFooterInSection:appIndexPath.section];
}
}
return ASFlowLayoutDefault(layout, footerReferenceSize, CGSizeZero); // UIKit documents this default to be CGSizeZero.
} else {
// In this case, we have an ASCellNode-based Supplementary Node as a footer. Ask it what size it wants to be.
return [self sizeForElement:element];
}

return [self sizeForElement:element];
}

- (UICollectionReusableView *)collectionView:(UICollectionView *)collectionView viewForSupplementaryElementOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
- (UICollectionReusableView *)collectionView:(UICollectionView *)collectionView
viewForSupplementaryElementOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
{
if ([_registeredSupplementaryKinds containsObject:kind] == NO) {
[self registerSupplementaryNodeOfKind:kind];
Expand Down
3 changes: 3 additions & 0 deletions Source/Details/ASDelegateProxy.m
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ - (BOOL)interceptsSelector:(SEL)selector
// handled by ASCollectionView node<->cell machinery
selector == @selector(collectionView:cellForItemAtIndexPath:) ||
selector == @selector(collectionView:layout:sizeForItemAtIndexPath:) ||
selector == @selector(collectionView:layout:insetForSectionAtIndex:) ||
selector == @selector(collectionView:layout:minimumLineSpacingForSectionAtIndex:) ||
selector == @selector(collectionView:layout:minimumInteritemSpacingForSectionAtIndex:) ||
selector == @selector(collectionView:layout:referenceSizeForHeaderInSection:) ||
selector == @selector(collectionView:layout:referenceSizeForFooterInSection:) ||
selector == @selector(collectionView:viewForSupplementaryElementOfKind:atIndexPath:) ||
Expand Down