react-native-immutable-list-view icon indicating copy to clipboard operation
react-native-immutable-list-view copied to clipboard

Support for section headers (SectionList?)

Open lucaslz2020 opened this issue 7 years ago • 15 comments

lucaslz2020 avatar May 23 '17 01:05 lucaslz2020

Eventually, yes! Currently it already supports VirtualizedList, which SectionList uses in the background. Feel free to submit a PR.


See discussion below for more details.

cooperka avatar May 23 '17 23:05 cooperka

Hi @cooperka I'm finally getting round to migrating from ListView and I'm struggling a little to work out if I can do it while continuing to use react-native-immutable-list-view. All my usage of react-native-immutable-list-view is with Immutable.Maps, rendered into sectioned lists. I've found this, but it seems to be empty, I guess because you haven't started work on it yet (based on your reply above). Reading the source code here:

https://github.com/cooperka/react-native-immutable-list-view/blob/b52a99c172b4b8c933bdc11f133cff765d646fb6/src/ImmutableVirtualizedList.js#L31

it looks like I can't do sectioned lists with react-native-immutable-list-view at all yet, even using the base ImmutableVirtualizedList. Is that correct?

Put another way, if I want to migrate my sectioned ImmutableListViews to use the new RN components, do I have to (currently) stop using this library?

Thanks for your time!

benadamstyles avatar Oct 09 '17 12:10 benadamstyles

Hi @Leeds-eBooks, I too would like to start using the new components with Maps. I'm currently in the process of refactoring to make it easier to support new types of lists. I don't want to promise anything, but I'd love to have it ready this week if things work out well. Feel free to "watch" this repo to get release updates. Thanks!

cooperka avatar Oct 10 '17 02:10 cooperka

@cooperka Thanks very much! I will happily wait, having taken a look at the VirtualizedList API I feel faintly nauseous... Let me know if there's anything I can do to help.

benadamstyles avatar Oct 10 '17 09:10 benadamstyles

@cooperka I have just finished implementing VirtualizedList directly in my app, using Immutable.Maps. I'm up for trying to port my implementation into your lib, but I have some questions and I don't want to plough time into a PR without checking you're happy with my approach first. What's the best way to discuss it? I'm on and offline, so instant messaging is not ideal – can we bat ideas back and forth on here?

benadamstyles avatar Jan 16 '18 16:01 benadamstyles

Sure, feel free to post here and I'll try to get back to you as quickly as I can.

Are you suggesting we don't support SectionList directly, but instead support Immutable.Map via VirtualizedList? Why wouldn't we just wrap SectionList (and maybe even WindowedList and all the other new lists)?

cooperka avatar Jan 16 '18 18:01 cooperka

Great, thanks. So I tried to make SectionList work, but the whole codebase of that component (including all the flow typing) is written to deal only with arrays and objects. The documentation for VirtualizedList actually specifically says:

In general, this should only really be used if you need more flexibility than FlatList provides, e.g. for use with immutable data instead of plain arrays.

Further, the docs for FlatList (although annoyingly not SectionList but I think that's just an omission) say:

For simplicity, data is just a plain array. If you want to use something else, like an immutable list, use the underlying VirtualizedList directly.

I took a look at the SectionList source code and it does this weird hack that I guess is the "canonical" way to achieve headers, which is to flatten your nested sections into a 1-dimensional list and keep a record of which indices should be section headers (and footers). It feels really hacky but if that's what facebook are doing then it must be the way. So it's not easy to handle for any given Map/List combo, but I think we can do it.

The alternative is that we do wrap SectionList but that will involve using lots of .toArray() and .toObject() and that doesn't feel very performant to me. SectionList doesn't provide any other way that I can find of grabbing items from an arbitrary data blob like VirtualizedList does.

benadamstyles avatar Jan 16 '18 18:01 benadamstyles

Hmm, I see what you mean. The API has changed significantly from ListView; I didn't realize SectionList took a totally different input prop. Transcribed here mostly for my own understanding:

ListView:

data: { section1: [row1, ...], section2: [], ... }

SectionList:

sections: [{ title: 'section1', data: [row1, ...] }, { title: 'section2', data: [] }, ...]

I understand it's more verbose on purpose, but I wouldn't want to store my data that way, I'd rather store it in a map with the keys being the titles (like the original ListView with sectionHeaders). For the sake of API consistency, though, we should probably do the same as SectionList if we call it ImmutableSectionList...

I'm starting to like your idea of simply supporting Immutable.Map within ImmutableVirtualizedList; I understand now why you went that route. I assume you're using the original data formatting conventions from ListView (instead of the new way)?

cooperka avatar Jan 17 '18 17:01 cooperka

To sum up: I see no need to support immutable data with SectionList specifically, because of the aforementioned API changes. What I do see is a need for section headers generally, which can be achieved using Immutable.Map with ImmutableVirtualizedList.

cooperka avatar Jan 17 '18 17:01 cooperka

My thoughts exactly! And yes, my aim in writing my implementation was to only swap my ImmutableListViews for VirtualizedLists, and not change anything else, especially my data structures.

The one obstacle I see however is that the way I've done it, and the way you suggest, using Immutable.Map keys as sectionHeaders, is that it rules out supporting sectionFooters in the official way. We could quite easily add a new prop of our own called renderSectionFooter that takes the section key as an argument, but that's not the way SectionList does it. If we're not calling it ImmutableSectionList then I guess that's fine.

The most important (and controversial?) piece of logic in my implementation is the function that flattens our nested immutable data structure (the Map of Maps) into something that VirtualizedList can receive. I built mine by looking at what SectionList does under the hood and making it a bit more functional:

// using an arrow function here for clarity but we'd use a method
data={(nestedMap) =>
  nestedMap.reduce(
    (map, subMap, key) => map.set(key, subMap).merge(subMap),
    Map()
  )
}

As you can see, what we end up with is a flat Map where section headers are interspersed with the sub rows in order at the head of each row. I think it would be trivial to handle sub-Lists as well.

For comparison, below is what SectionList does. Rather than eagerly transforming the input data structure into a flat one, it does the following every time getItem is called:

function getItem(sections: ?$ReadOnlyArray<Item>, index: number): ?Item {
  if (!sections) {
    return null;
  }
  let itemIdx = index - 1;
  for (let ii = 0; ii < sections.length; ii++) {
    if (itemIdx === -1 || itemIdx === sections[ii].data.length) {
      // We intend for there to be overflow by one on both ends of the list.
      // This will be for headers and footers. When returning a header or footer
      // item the section itself is the item.
      return sections[ii];
    } else if (itemIdx < sections[ii].data.length) {
      // If we are in the bounds of the list's data then return the item.
      return sections[ii].data[itemIdx];
    } else {
      itemIdx -= sections[ii].data.length + 2; // Add two for the header and footer
    }
  }
  return null;
}

Basically, for each index (and it's a 1-dimensional index) up to the total supplied by the getItemCount prop, they deeply traverse the structure until they find the item, or the sectionHeader / sectionFooter if that's what the index is pointing to. I assume they took this approach because they don't have the luxury of immutability.

benadamstyles avatar Jan 17 '18 18:01 benadamstyles

That sounds about right -- if the list has infinite sections, they couldn't possibly flatten them all. Yay, functional programming!

It certainly needs some testing, but I like your solution. Would you mind submitting a PR when you have time? I will likely end up tweaking things to play around with it so don't worry too much about making it perfect.

cooperka avatar Jan 17 '18 19:01 cooperka

Ok great, will do. Might be a few days, but I'll make a start right away.

benadamstyles avatar Jan 17 '18 19:01 benadamstyles

Hey @cooperka, I have one more little question now that I've made a (slightly delayed) start on this. We need some way to choose whether to treat immutableData as a flat list or a sectioned list. Do you think we can assume that if the user passes a Map into the immutableData prop that we should always treat it as a list with section headers? Or do we need to choose whether to apply section headers some other way? The options I can think of are:

if (Map.isMap(immutableData))
if (isImmutable(immutableData.first()))
if (this.props.treatAsSections === true)

benadamstyles avatar Jan 23 '18 11:01 benadamstyles

See my PR: #34

benadamstyles avatar Jan 23 '18 13:01 benadamstyles

Awesome! I left comments there addressing the above.

cooperka avatar Jan 29 '18 22:01 cooperka