GriddleGriddle / Griddle

Simple Grid Component written in React
http://griddlegriddle.github.io/Griddle/
MIT License
2.5k stars 378 forks source link

Total number of pages via 'maxPageSelector' is incorrect #788

Closed MichaelDeBoey closed 6 years ago

MichaelDeBoey commented 6 years ago

Griddle version

1.11.1

Problem

When I'm implementing the Navigation myself, by copying the original and just adding

<div>of {maxPages}</div>

The shown maxPages will always result in 1, like you can see in the CodeSandbox instead of the 10 that's shown in the dropdown

The maxPageSelector that I use is the same that gets called in PageDropdownContainer, so am I still doing something wrong? 😕

CC: @dahlbyk

dahlbyk commented 6 years ago

You're using the default maxPageSelector, which requires that you manage pageProperties.recordCount yourself. You want to use LocalPlugin.selectors.maxPageSelector` instead: https://codesandbox.io/s/zl0r5jz7nl.

It would be more correct to get selectors from React context (example), but we don't consistently follow that pattern even within the default components.

For the record, the recommended Griddle pattern for what you're doing would put the connect inside PaginationContainer and let Pagination focus only on rendering from props.

MichaelDeBoey commented 6 years ago

Doh! 😓 Totally missed out on the LocalPlugin 😓 Thanks for bringing that up! 🙂

We don't want to use the React context since that's not recommended, but thanks for the tip 😉

dahlbyk commented 6 years ago

We don't want to use the React context since that's not recommended, but thanks for the tip

To be clear, Griddle uses context extensively for making configuration (e.g. composed components from plugins/props) available to child components (Parent-Child Coupling). You probably shouldn't use it for your own app's infrastructure, but it's often the correct way to customize Griddle (see #740, #743).