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

WIP: Updating IGListGridCollectionViewLayout to be more flexible #423

Closed
wants to merge 9 commits into from

Conversation

zhubofei
Copy link

@zhubofei zhubofei commented Jan 17, 2017

Changes in this pull request

Updating the grid layout to fit more general cases.

Closes #263

TODO

  • Add support for multi-item sections.
  • Add unit test for multi-item sections.
  • Add IGListGridCollectionViewLayoutAlignment option. Allow user to set alignment for the layout.
  • Add unit test for alignments.
  • Add support for horizontal scrolling.
  • Add unit test for horizontal mode.
  • Add support for section inset.
  • Add unit test for section inset.
  • Add attributes cache.
  • Update Changelog

@coveralls
Copy link

coveralls commented Jan 17, 2017

Coverage Status

Coverage decreased (-0.7%) to 97.388% when pulling 582f11a on zhubofei:center-align into 8d74b8f on Instagram:master.

An option for how to align items in grid layout.
*/
typedef NS_ENUM(NSInteger, IGListGridCollectionViewLayoutAlignment) {
IGListGridCollectionViewLayoutAlignDefault,
Copy link
Contributor

Choose a reason for hiding this comment

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

  • what does Default mean? i would expect to see Left, Right, Center...
  • nit: can we spell-out Alignment for each enum case? (follows conventions)

@facebook-github-bot
Copy link
Contributor

@zhubofei updated the pull request - view changes

@zhubofei zhubofei self-assigned this Jan 18, 2017
@coveralls
Copy link

coveralls commented Jan 19, 2017

Coverage Status

Coverage decreased (-0.7%) to 97.322% when pulling 7bcf15c on zhubofei:center-align into 8d74b8f on Instagram:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 97.322% when pulling 7bcf15c on zhubofei:center-align into 8d74b8f on Instagram:master.

@facebook-github-bot
Copy link
Contributor

@zhubofei updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

@zhubofei updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

@zhubofei updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

@zhubofei updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

@zhubofei updated the pull request - view changes

@rnystrom
Copy link
Contributor

@zhubofei heads up that we are talking internally about adding a layout we built for IG to IGLK. It does all of this and is wicked fast. If we do, we'll probably decide to stick w/ that one layout. Just want you to know so you don't spend tons of time on this just for us to replace it.

Sorry if this causes some thrash!

@zhubofei
Copy link
Author

@rnystrom This is a great news! I'm actually really worried about the performance of my implementation and cannot come up with a better layout algorithm. Really curious about the wicked fast algorithm from you guys, can't wait to see it!

@zhubofei
Copy link
Author

@rnystrom The new layout is superb! I actually thought about rewriting the whole thing with objective-c++ before, but lack the motivation to do so 😂.

I'll close this one then.

@zhubofei zhubofei closed this Jan 27, 2017
facebook-github-bot pushed a commit that referenced this pull request Feb 10, 2017
Summary:
Working on porting our collection view layout to IGListKit. I'm doing this because its a solid layout, and we just finished preparing it to work with inline sections. It is designed to work in tandem with IGListKit, so we're adding it.

This is still a WIP as I add more tests, but I'd love as much feedback as possible.

Aside from the glob of header documentation, this has the following features:

- Infinite sections that each have infinite items. Sections and items can fall inline. When they break the width of their container they will fall on the next row.
- Sections can have their own insets, line spacing, and interitem spacing.
- Sticky header support! When you use headers, it will always newline the section.
- Maximum width with a border decoration view
  - Use this to pinch in your content on larger devices

Followup to #423

- [ ] ~~Move decoration view support to delegate~~ removed
- [x] Unit test changing [top y sticky inset](https://coveralls.io/builds/9977284/source?filen
Closes #450

Reviewed By: jessesquires

Differential Revision: D4521797

Pulled By: rnystrom

fbshipit-source-id: 20b36ae573d38ca3125a6f3d5faec181c290ab94
facebook-github-bot pushed a commit that referenced this pull request Feb 11, 2017
Summary:
Working on porting our collection view layout to IGListKit. I'm doing this because its a solid layout, and we just finished preparing it to work with inline sections. It is designed to work in tandem with IGListKit, so we're adding it.

This is still a WIP as I add more tests, but I'd love as much feedback as possible.

Aside from the glob of header documentation, this has the following features:

- Infinite sections that each have infinite items. Sections and items can fall inline. When they break the width of their container they will fall on the next row.
- Sections can have their own insets, line spacing, and interitem spacing.
- Sticky header support! When you use headers, it will always newline the section.
- Maximum width with a border decoration view
  - Use this to pinch in your content on larger devices

Followup to #423

- [ ] ~~Move decoration view support to delegate~~ removed
- [x] Unit test changing [top y sticky inset](https://coveralls.io/builds/9977284/source?filen
Closes #484

Differential Revision: D4547760

Pulled By: rnystrom

fbshipit-source-id: 879e2da16eb78bb6a90967e77d9ad0bbf7c69594
@ShaneQi
Copy link

ShaneQi commented Feb 20, 2017

@zhubofei Since pr#484 and pr#450 are already closed, do you think we can expect this pull request reopen?

Anyway, I have some comments:

  • Alignment layout is very useful, I love the idea, and this will definitely help me out in some cases.
  • Is it possible that different sections have different IGListGridCollectionViewLayoutAlignment options? (sounds like tons of work to implement this)

Thanks!

@zhubofei
Copy link
Author

zhubofei commented Feb 20, 2017

@ShaneQi We need @rnystrom 's opinion on the alignment option. I'm happy to work on it if he approves this option.

Is it possible that different sections have different IGListGridCollectionViewLayoutAlignment options? (sounds like tons of work to implement this)

This one, though, is too complicated for a general layout. I suggest you subclass the layout and customize it as needed.

@ShaneQi
Copy link

ShaneQi commented Feb 20, 2017

Thanks for the reply and all the contribution you've done!

@rnystrom
Copy link
Contributor

@ShaneQi sadly we replaced this layout with #484. Agree w/ @zhubofei that alignment might just be beyond the scope of IGListKit and require a more comprehensive UICollectionViewLayout entirely (there are some great libs out there already).

If someone has an elegant way to extend our layout, I'm very into a PR to try some stuff out.

@ShaneQi
Copy link

ShaneQi commented Feb 20, 2017

@rnystrom @zhubofei I have to say you are right, aligning cells in some specific sections is too specific. One should subclass UICollectionViewLayout to achieve that.

BTW, thanks for bringing new layout to us! ❤️

@zhubofei zhubofei deleted the center-align branch June 29, 2018 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants