Show header when section item is empty #1129

marcuswu0814 · 2018-03-18T12:41:48Z

Changes in this pull request

Issue fixed: #1117

I adding a new constructor for making a IGListCollectionViewLayout instance that can always show sticky header although section data is empty.

It's working well and this is demo project.

But I'm not sure is any good way to let this much more readability. Is there any good advice?

const CGRect headerBounds = (self.scrollDirection == UICollectionViewScrollDirectionVertical) ?
                CGRectMake(insets.left,
                        (itemCount == 0) ? CGRectGetMaxY(rollingSectionBounds) : CGRectGetMinY(rollingSectionBounds) - headerSize.height,
                        paddedLengthInFixedDirection,
                        headerSize.height) :
                CGRectMake((itemCount == 0) ? CGRectGetMaxX(rollingSectionBounds) : CGRectGetMinX(rollingSectionBounds) - headerSize.width,
                        insets.top,
                        headerSize.width,
                        paddedLengthInFixedDirection);

Checklist

All tests pass. Demo project builds and runs.
I added tests, an experiment, or detailed why my change isn't tested.
I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
I have reviewed the contributing guide

For making a layout that can still show sticky header when data is empty

…ny items

facebook-github-bot · 2018-03-18T12:43:45Z

@marcuswu0814 has updated the pull request. View: changes

rnystrom · 2018-03-22T01:59:37Z

Source/IGListCollectionViewLayout.h

+- (instancetype)initWithStickyHeaders:(BOOL)stickyHeaders
+                  showHeaderWhenEmpty:(BOOL)showHeaderWhenEmpty
+                      topContentInset:(CGFloat)topContentInset
+                        stretchToEdge:(BOOL)stretchToEdge;


Could we actually just include this as a mutable @property BOOL showHeaderWhenEmpty? Trying to prevent some initializer creep here, ha!

Sure, I will rolling it back and extract the property showHeaderWhenEmpty to public.

rnystrom · 2018-03-22T02:00:50Z

Tests/IGListCollectionViewLayoutTests.m

+
+    IGAssertEqualFrame([self headerForSection:0].frame, 0, 0, 0, 0);
+    IGAssertEqualFrame([self headerForSection:1].frame, 0, 0, 0, 0);
+}


❤️ tests!

rnystrom · 2018-03-22T02:01:27Z

Source/IGListCollectionViewLayout.mm

                        insets.top,
                        headerSize.width,
                        paddedLengthInFixedDirection);

        _sectionData[section].headerBounds = headerBounds;
-
+
+        if (itemCount == 0) {


Could we capture itemCount == 0 in a variable?

const BOOL itemsEmpty = itemCount == 0;

rnystrom · 2018-03-22T02:02:54Z

Source/IGListCollectionViewLayout.mm

        const CGRect headerBounds = (self.scrollDirection == UICollectionViewScrollDirectionVertical) ?
                CGRectMake(insets.left,
-                        CGRectGetMinY(rollingSectionBounds) - headerSize.height,
+                        (itemCount == 0) ? CGRectGetMaxY(rollingSectionBounds) : CGRectGetMinY(rollingSectionBounds) - headerSize.height,


What happens if itemCount == 0 but showHeaderWhenEmpty is NO?

When I trying this, found another bug and note it below.

1. Extract the property to public, remove initializer 2. Capture condition `itemCount == 0` to const var. 3. Modified the test case from using initializer setup to using property

facebook-github-bot · 2018-03-22T07:14:24Z

@marcuswu0814 has updated the pull request. View: changes

Keep the `rollingSectionBounds` and `headerBounds` original behavior. Adding condition `itemsEmpty && self.showHeaderWhenEmpty` to do my job.

facebook-github-bot · 2018-03-22T08:47:58Z

@marcuswu0814 has updated the pull request. View: changes

marcuswu0814 · 2018-03-22T09:41:54Z

I think the bug is not cause by my change.

The following demo is using this project.

In branch `IG/release`

Before any thing change, used release version install by current pod latest version:

Both header and footer had some bug.

In branch `IG/master`

The footer work well, maybe fixed it #1094, but header got some bug like release version.

In my change, branch `marcuswu0814/feature`

If set property showHeaderWhenEmpty to NO and the header position will getting wrong.

So, In current master branch, if section item is empty and using sticky header, sticky header position will getting some bugs.

In previous version, the test didn't cover this specail case: Three sections, 0 and 2 had itmes and header, 1's items is empty. In this case, the header postion will be wrong. Change this test case for cover it.

…other section's header postion be wrong bugs

facebook-github-bot · 2018-03-22T12:29:13Z

@marcuswu0814 has updated the pull request. View: changes

marcuswu0814 · 2018-03-22T12:37:23Z

I fixed this bug on my branch latest commit, and the old test case didn't cover this bug, I already modified the test case and pass it. Now, avoid this bug! 😎

Need open another issue to track this?
And if having any new issue, feel free to tell me, thanks. 😅

facebook-github-bot · 2018-03-22T17:26:14Z

@marcuswu0814 has updated the pull request. View: changes

marcuswu0814 · 2018-03-27T13:54:30Z

Hi @rnystrom, will this possible be merged in 3.3.0 release?

facebook-github-bot · 2018-03-27T18:37:51Z

@marcuswu0814 has updated the pull request. View: changes

facebook-github-bot · 2018-03-27T18:40:20Z

@marcuswu0814 has updated the pull request. View: changes

facebook-github-bot · 2018-04-09T13:29:18Z

@marcuswu0814 has updated the pull request. View: changes

facebook-github-bot

@rnystrom is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

marcuswu0814 added 4 commits March 18, 2018 19:43

Implement if item is empty, SupplementaryView still show feature

3b2f253

Adding a new constructor in IGListCollectionViewLayout

819f034

For making a layout that can still show sticky header when data is empty

Add a test case for showing sticky header when section had no any items

ef63c86

Add a test case for sticky header should hidden when section had no a…

704e013

…ny items

facebook-github-bot added the CLA Signed label Mar 18, 2018

Adding CHANGELOG

66e271c

marcuswu0814 changed the title ~~Show header when section item empty~~ Show header when section item is empty Mar 18, 2018

marcuswu0814 mentioned this pull request Mar 18, 2018

Section is invisible when numberOfItems is 0 #1117

Closed

rnystrom suggested changes Mar 22, 2018

View reviewed changes

Modified for code review suggestion

75b7980

1. Extract the property to public, remove initializer 2. Capture condition `itemCount == 0` to const var. 3. Modified the test case from using initializer setup to using property

Remove remove initializer in .m file

149351d

Keep the `rollingSectionBounds` and `headerBounds` original behavior. Adding condition `itemsEmpty && self.showHeaderWhenEmpty` to do my job.

marcuswu0814 added 2 commits March 22, 2018 20:23

Modified the test case when don't what to show header

4d35f63

In previous version, the test didn't cover this specail case: Three sections, 0 and 2 had itmes and header, 1's items is empty. In this case, the header postion will be wrong. Change this test case for cover it.

Fixed if didn't using sticky header and some section items is empty, …

fb4c59a

…other section's header postion be wrong bugs

Remove unused code

09314eb

add docs about default value

d4d0416

remove extra parens

ea0501e

rnystrom added this to the 3.3.0 milestone Mar 27, 2018

Merge branch 'master' into feature/showHeaderWhenSectionItemEmpty

ce7f439

facebook-github-bot reviewed Apr 9, 2018

View reviewed changes

facebook-github-bot closed this in e49c94b Apr 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show header when section item is empty #1129

Show header when section item is empty #1129

marcuswu0814 commented Mar 18, 2018 •

edited

Loading

facebook-github-bot commented Mar 18, 2018

rnystrom Mar 22, 2018

marcuswu0814 Mar 22, 2018 •

edited

Loading

rnystrom Mar 22, 2018

marcuswu0814 Mar 22, 2018

rnystrom Mar 22, 2018

marcuswu0814 Mar 22, 2018

rnystrom Mar 22, 2018

marcuswu0814 Mar 22, 2018

facebook-github-bot commented Mar 22, 2018

facebook-github-bot commented Mar 22, 2018

marcuswu0814 commented Mar 22, 2018 •

edited

Loading

facebook-github-bot commented Mar 22, 2018

marcuswu0814 commented Mar 22, 2018 •

edited

Loading

facebook-github-bot commented Mar 22, 2018

marcuswu0814 commented Mar 27, 2018

facebook-github-bot commented Mar 27, 2018

facebook-github-bot commented Mar 27, 2018

facebook-github-bot commented Apr 9, 2018

facebook-github-bot left a comment

Show header when section item is empty #1129

Show header when section item is empty #1129

Conversation

marcuswu0814 commented Mar 18, 2018 • edited Loading

Changes in this pull request

Checklist

facebook-github-bot commented Mar 18, 2018

rnystrom Mar 22, 2018

Choose a reason for hiding this comment

marcuswu0814 Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

rnystrom Mar 22, 2018

Choose a reason for hiding this comment

marcuswu0814 Mar 22, 2018

Choose a reason for hiding this comment

rnystrom Mar 22, 2018

Choose a reason for hiding this comment

marcuswu0814 Mar 22, 2018

Choose a reason for hiding this comment

rnystrom Mar 22, 2018

Choose a reason for hiding this comment

marcuswu0814 Mar 22, 2018

Choose a reason for hiding this comment

facebook-github-bot commented Mar 22, 2018

facebook-github-bot commented Mar 22, 2018

marcuswu0814 commented Mar 22, 2018 • edited Loading

In branch IG/release

In branch IG/master

In my change, branch marcuswu0814/feature

facebook-github-bot commented Mar 22, 2018

marcuswu0814 commented Mar 22, 2018 • edited Loading

facebook-github-bot commented Mar 22, 2018

marcuswu0814 commented Mar 27, 2018

facebook-github-bot commented Mar 27, 2018

facebook-github-bot commented Mar 27, 2018

facebook-github-bot commented Apr 9, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

marcuswu0814 commented Mar 18, 2018 •

edited

Loading

marcuswu0814 Mar 22, 2018 •

edited

Loading

marcuswu0814 commented Mar 22, 2018 •

edited

Loading

In branch `IG/release`

In branch `IG/master`

In my change, branch `marcuswu0814/feature`

marcuswu0814 commented Mar 22, 2018 •

edited

Loading