Add support for custom pagination

[prayerslayer]

Hello,

I'm planning to use your nice Elasticsearch UI for a web project, but I need custom pagination (issue #127).

Changes:

• Add a prop renderPagination. It should be a function that receives pagination-related data and should return a JSX element
• Make size part of ReactiveList state, because it's relatively common that you let users choose the page size, and it's more convenient for developers if they can do it from their custom pagination component.

This seems to work in a storybook, but I'm wondering if the diff you see is all there is to it.

• There don't seem to be tests I should add?
• The playground is a separate repo, do I make a PR to add the storybook there?
• Do I need to add support for React Native etc. too? Didn't look at those yet.
• There's a high chance I missed something

Anyways I hope you find it helpful and maybe we can merge it at some point :)

[jyash97]

Hey, @prayerslayer thanks for the PR. Have a few questions:

1. What is the intention to introduce setSize function?
2. At L700 I think we should only check for renderPagination prop and not the type as it would be handled using prop type. Thoughts ?

[prayerslayer]

Hey!

1. I was thinking about a pagination component where you can navigate between pages and also set the page size, which I think is not too uncommon. It would be convenient if the pagination render function provides a function to do so, otherwise - to use the existing prop on ReactiveList (?) - you’d need to set some state outside of that component. That’d make things harder to read and reason about, in my opinion.
2. Sure, if you prefer it like that, i don’t mind changing.

Thanks for taking the time!

[prayerslayer]

I fixed the second point you mentioned @jyash97. As for the first one, I'd prefer the custom pagination component to be written like this (from my local storybook):

<ReactiveListDefault
        pagination={boolean("pagination", true)}
        renderPagination={({ pages, totalPages, currentPage, setPage, setSize }) => {
          const selectPage = Number.isFinite(pages) && (
            <select onChange={e => setPage(parseInt(e.target.value, 10))}>
              {new Array(pages).fill(0).map((_, i) => (
                <option value={i}>{i + 1}</option>
              ))}
            </select>
          );
          const selectPageSize = (
            <div>
              Page size:{" "}
              <select onChange={e => setSize(parseInt(e.target.value, 10))}>
                <option selected value={5}>5</option>
                <option value={10}>10</option>
                <option value={25}>25</option>
                <option value={50}>50</option>
              </select>
            </div>
          );
          return (
            <div>
              Page {currentPage + 1}/{pages} {selectPage} {selectPageSize}
            </div>
          );
        }}
        paginationAt={select(
          "paginationAt",
          { bottom: "bottom", top: "top", both: "both" },
          "bottom"
        )}
        size={number('size', 5)}
        pages={number("pages", 5)}
        title="Meetups"
      />

I hope that makes things more clear. Is there anything else I'd need to do to get this merged?

Also I was thinking about how one would write custom pagination, but with the additional requirement that it'd be different whether it's at the top or bottom. Think e.g. page size selection and result count at the top, page selection at the bottom. How would that work? I think the most straightforward solution would be pass an additional property paginationPosition (can be "top" or "bottom") to the custom pagination component. Would that be something you're comfortable merging too?

[jyash97]

Hey @prayerslayer
Thanks for the explanation 💯. Will check the functionality once and update if something is missing or required.

For the custom pagination size, we can cover it in a different PR after this is merged. But I think that can be achieved using renderResultStats.

[prayerslayer]

Sure, thank you very much :) Let me know if I can help.

[jyash97]

Hey @prayerslayer
I was trying to achieve the size switcher and I was able to achieve this without any additional requirement. ReactiveList has a prop called size and we can manage size in a state variable and pass it in size prop.

Check this codesandbox example: https://codesandbox.io/s/reactivelist-1czg3

If this looks good, we can remove setSize.

[prayerslayer]

Hi!

I was trying to achieve the size switcher and I was able to achieve this without any additional requirement. ReactiveList has a prop called size and we can manage size in a state variable and pass it in size prop.

Yeah, I realized that before, but I'm not sure I like it. See my previous comment:

It would be convenient if the pagination render function provides a function to do so, otherwise - to use the existing prop on ReactiveList (?) - you’d need to set some state outside of that component.

Pagination state (current page, number of pages, page size) is now distributed among ReactiveBase (current page, number of pages) and its parent (page size). I think it'd be good practice to keep related data and functionality in the same place. If the concern is that other code might be depending on the current page size I'd add an onChange callback for it.

But maybe I'm trying to change too much here. I don't really agree with how my code will look like, but I think a renderPagination plus the renderResultStats misuse will let me do what I want. I'll revert the setSize change.

[prayerslayer]

Done @jyash97 :)

[jyash97]

The changes look good only a small change is needed. 💯

[prayerslayer]

Thanks! I checked all size instances in ReactiveList, there should not be any more leftovers :)

[jyash97]

Awesome 💯
Thanks for the PR. Really clean implementation 💥
@lakhansamani @bietkul If this looks good we can merge it.

[bietkul]

PR looks good, @jyash97 Can you please update the docs then we can merge it.

[jyash97]

Added here: appbaseio/reactive-manual#182

[prayerslayer]

Awesome, thanks! Do you know when this will be available in a release? :) Asking because the docs are already updated 😅

[jyash97]

Hey @prayerslayer

It will be released soon and I will update you over here once the new version is out.

Thanks for the contribution 💯

[bietkul]

@prayerslayer It's been released now.

[prayerslayer]

Very nice, thanks a ton!