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

Support for section headers (SectionList?) #25

Open
lucaslz2020 opened this issue May 23, 2017 · 15 comments
Open

Support for section headers (SectionList?) #25

lucaslz2020 opened this issue May 23, 2017 · 15 comments

Comments

@lucaslz2020
Copy link

No description provided.

@cooperka
Copy link
Owner

cooperka commented May 23, 2017

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.

@benadamstyles
Copy link

benadamstyles commented Oct 9, 2017

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:

return new Error(`Invalid prop ${propName} supplied to ${componentName}: Support for Immutable.Map is coming soon. For now, try an Immutable List, Set, or Range.`);

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!

@cooperka
Copy link
Owner

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!

@benadamstyles
Copy link

@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
Copy link

@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?

@cooperka
Copy link
Owner

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)?

@benadamstyles
Copy link

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.

@cooperka
Copy link
Owner

cooperka commented Jan 17, 2018

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 cooperka changed the title Will support SectionList? Support for section headers (SectionList?) Jan 17, 2018
@cooperka cooperka reopened this Jan 17, 2018
@cooperka
Copy link
Owner

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.

@benadamstyles
Copy link

benadamstyles commented Jan 17, 2018

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.

@cooperka
Copy link
Owner

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.

@benadamstyles
Copy link

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

@benadamstyles
Copy link

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
Copy link

See my PR: #34

@cooperka
Copy link
Owner

Awesome! I left comments there addressing the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants