Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

[ASTableNode/ASCollectionNode] Inversion#2891

Merged
Adlai-Holler merged 6 commits intofacebookarchive:masterfrom
aaronschubert0:inversion
Jan 20, 2017
Merged

[ASTableNode/ASCollectionNode] Inversion#2891
Adlai-Holler merged 6 commits intofacebookarchive:masterfrom
aaronschubert0:inversion

Conversation

@aaronschubert0
Copy link
Copy Markdown
Contributor

@aaronschubert0 aaronschubert0 commented Jan 12, 2017

Implements #2837 on ASTableNode and ASCollectionNode. I've tried to keep the API minimal. Also added an example app, since the user will have to adjust the contentInset of their ASTable/CollectionNode for the inversion to work properly.

Documentation: AsyncDisplayKit/Documentation#78

@appleguy

@aaronschubert0 aaronschubert0 changed the title Inversion [ASTableNode/ASCollectionNode] Inversion Jan 12, 2017
@hannahmbanana
Copy link
Copy Markdown
Contributor

Thank you for including a sample project and documentation @aaronschubert0!

Copy link
Copy Markdown
Contributor

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Thanks for a diff! This technique is super-powerful so it'll be great to support natively.

Comment thread AsyncDisplayKit/ASCollectionNode.mm Outdated
- (void)setInverted:(BOOL)inverted
{
self.view.dataController.inverted = inverted;
self.transform = inverted ? CATransform3DMakeRotation(M_PI, 0, 0, 1.0) : CATransform3DIdentity;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use CATransform3DMakeScale(1, -1, 1) instead of rotation. It's cleaner because π can't be represented perfectly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Awesome, will change it to that, thanks.

}

if (_inverted) {
context.node.transform = CATransform3DMakeRotation(M_PI, 0, 0, 1.0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be better to put this code in -[ASCollectionView/ASTableView dataController:nodeBlockAtIndexPath:] near the bottom along with the other customization that gets applied to the node, so that it's run exactly once per node, so the data controller doesn't have to be aware of the inversion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense, I had opted to avoid this initially so that I didn't have to duplicate code, but for the sake of two extra lines of code, it would feel nice not to have the ASDataController be involved.

@Adlai-Holler
Copy link
Copy Markdown
Contributor

Jenkins, test this please

@hannahmbanana
Copy link
Copy Markdown
Contributor

Build error:

AsyncDisplayKit/ASCollectionNode.mm:221:1: ivar '_inverted' which backs the property is not referenced in this property's accessor [-Werror,-Wunused-property-ivar]
11:22:06 
11:22:06 - (void)setInverted:(BOOL)inverted

@aaronschubert0
Copy link
Copy Markdown
Contributor Author

@Adlai-Holler Made those changes!

@hovox
Copy link
Copy Markdown

hovox commented Jan 17, 2017

+1 For this.

1 similar comment
@davitPiloyan93
Copy link
Copy Markdown

+1 For this.

@NikoSahradyan
Copy link
Copy Markdown

+1 For this. good job

@Adlai-Holler
Copy link
Copy Markdown
Contributor

Jenkins, add to whitelist

- (void)setInverted:(BOOL)inverted
{
self.view.inverted = inverted;
self.transform = inverted ? CATransform3DMakeScale(1, -1, 1) : CATransform3DIdentity;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to implement this flag the same way we implement delegate – pass to the pending state or to the view, and that'll make the unused _inverted ivar disappear.

@hasmikvardanyan
Copy link
Copy Markdown

+1

@aaronschubert0
Copy link
Copy Markdown
Contributor Author

@Adlai-Holler Added the logic to the pending state instead, should be good to go now.

ASDisplayNodeAssert([self isNodeLoaded], @"ASCollectionNode should be loaded if pendingState doesn't exist");
self.view.inverted = inverted;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You'll need to override the accessor - (BOOL)inverted as well (see -delegate). The idea is to get rid of the synthesized _inverted ivar, because it's not used and that's making the compiler 💣

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, I've added the accessor

@Adlai-Holler
Copy link
Copy Markdown
Contributor

Awesome, thanks! Here we go folks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants