Section headers

[benadamstyles]

This is just my initial commit – it would be really helpful if @cooperka you could take a glance at this before I continue. I have had to make some possibly controversial decisions, most notably introducing a setState in componentDidUpdate – for which I had to make a change in .eslintrc.

I welcome any comments, now matter how challenging they are!

[cooperka]

I made a few minor style tweaks.

I think I might be passing in the wrong data format because I get TypeError: Expected [K, V] tuple at utils.flattenMap at reduce (this happens on both your original branch and after my changes). Any thoughts? Here's my example app diff to try it out:

diff --git a/example/index.js b/example/index.js
index 3fd0fe8..e5abcac 100644
--- a/example/index.js
+++ b/example/index.js
@@ -1,7 +1,7 @@
 import { AppRegistry } from 'react-native';

 // Choose one:
-import App from './src/ImmutableListViewExample';
-// import App from './src/ImmutableVirtualizedListExample';
+// import App from './src/ImmutableListViewExample';
+import App from './src/ImmutableVirtualizedListExample';

 AppRegistry.registerComponent('ImmutableListViewExample', () => App);
diff --git a/example/src/ImmutableVirtualizedListExample.js b/example/src/ImmutableVirtualizedListExample.js
index 5465c12..e5996c7 100644
--- a/example/src/ImmutableVirtualizedListExample.js
+++ b/example/src/ImmutableVirtualizedListExample.js
@@ -20,18 +20,23 @@ import utils from './utils';
  *
  */
 function ImmutableVirtualizedListExample() {
+  const sectionData = Immutable.fromJS({
+    sec1: [1, 2, 3],
+  });
+
   return (
     <GenericListExample
       ListComponent={ImmutableVirtualizedList}
       listComponentProps={{
         renderItem: utils.renderItem,
+        renderSectionHeader: utils.renderSectionHeader,
         keyExtractor: utils.trivialKeyExtractor,
       }}

-      initialDataA={Immutable.List(['Simple', 'List', 'of', 'Items'])}
+      initialDataA={sectionData}
       dataMutatorA={(data) => data.set(3, 'This value was changed!')}

-      initialDataB={Immutable.Range(1, 100)}
+      initialDataB={sectionData}
       dataMutatorB={(data) => data.toSeq().map((n) => n * 2)}
     />
   );

[benadamstyles]

Thanks for making these tweaks. I've been unexpectedly busy this week which is why I haven't had a chance to continue working on this, but it's really high on my to-do list! At the latest, I have 17th Feb free to work on this. Got to add tests, mainly.

[benadamstyles]

Oh, just seen your question! Will take a look at this asap. It's not obvious to me off the top of my head.

[cooperka]

It's all good, I know how life can be. Tests would be great, and will probably resolve my issue! Either by helping me understand the data format or by catching a bug ;) Take your time!

[benadamstyles]

Ok it's because my usage of merge in flattenMap only handles a Map with Map rows, rather than List rows. Will work on this next!

[benadamstyles]

Ok think this is working now!

[benadamstyles]

Just catching up with this – does anything remain to be done? Does it need more tests do you think?

[cooperka]

Hmm, it still doesn't seem to work with the example app using the code diff I pasted above. The list is simply blank (though no errors anymore). It also gets into a strange state when loading and erroring (see screenshot). Do you see the same thing as me?

[benadamstyles]

@cooperka Ah yes, I never got round to testing with a live app – just doing that now. Is there a nicer way to make changes in the package and see it updated in the example app than killing the app and running yarn again?

[cooperka]

In an earlier version of RN + yarn you could simply npm install file:.. which took about 5 seconds and it would live update. This no longer works unfortunately.

It might work to yarn add file:.., restart the packager, and simply refresh the app (ctrl+R on ios or R+R on android).

[cooperka]

I know there are other automated solutions; if you're familiar with one feel free to recommend it. I've never needed something like that until now since the npm trick used to work.

[cooperka]

The items display now, but it shows 123 as the section header instead of sec1 as expected. There are also still 4 "Loading" rows. The code in GenericListExample might need to be updated to fix this; I haven't diagnosed any further than just visually.

[benadamstyles]

Yeah sorry, got to change something in the example app's code. Coming right up 🙂

[benadamstyles]

Ah I think I remember now why I didn't originally pass the whole info object to renderSectionHeader(): I think I took my cue from the test-utils renderers. In the row renderer, you expect an info object with a item property, but in the sectionHeader renderer, you expect the sectionData directly (i.e. not wrapped in an info object. Should I revert my last commit, or refactor all code throughout the test utils and example app to read the sectionData as being at info.item (as it is for row data)?

[cooperka]

My intuition is that we should use the same data format conventions as the original ListView, but the function arguments should be formatted like VirtualizedList for consistency.

Eventually I'd love to release ImmutableSectionList as a counterpart to SectionList, but that would be more work since it's a very different API with extra features.

My recommendation for this PR (feel free to suggest changes):

// This data:
Immutable.fromJS({
  sec1: [1, 2, 3],
})

// Or, equivalently:
Immutable.fromJS({
  sec1: { r1: 1, r2: 2, r3: 3 },
})

Would render as sec1 section with 3 items: 1, 2, 3.

The methods would be called as follows:

renderSectionHeader({ sectionData, sectionID })
renderItem({ item, index })

So test-utils.js will need to be updated to include a renderVirtualizedSectionHeader method with these arguments (the existing renderSectionHeader is for ImmutableListView only).

What do you think?

[taranda]

@cooperka and @benadamstyles: Have you made any progress on this? I would love to see section headers implemented, and I like the shape of the input data.

Do you need any help pushing this over the goal line?

[alexiri]

We could really use this functionality as well. We're currently using ImmutableListView, but we're seeing some performance issues.