Trigger griddle callbacks on next and previous pagination

[JesseFarebro]

Griddle major version

1.8.0

Changes proposed in this pull request

I think the actions should still be triggered for pagination regardless of the data being local.

Why these changes are made

Trying to mutate the state from a reducer on these events.

Are there tests?

No.

[dahlbyk]

I wonder if it would be better to move getNext/getPrevious out of the container (for default and LocalPlugin) down into their corresponding enhancers? Something like:

  mapProps(({ events: { getNext, onNext }, ...props }) => ({
    ...props,
    onClick: combineHandlers([onNext, getNext, props.onClick]),
  }))

[JesseFarebro]

@dahlbyk that is kind of what I was thinking when I first looked at this as well. Should I update the PR to reflect this?

[ryanlanciaux]

I think that would be good to move if possible.

Maybe a discussion for future Griddle versions too -- the breakdown between enhancers and containers and if it is necessary. 🤔

[JesseFarebro]

@ryanlanciaux @dahlbyk PR updated now with the actions in the enhancer.

@ryanlanciaux This would be a good conversation to have. One of the big issues I'm facing right now is composing a grid with multiple plugins.

[dahlbyk]

@ryanlanciaux: Maybe a discussion for future Griddle versions too -- the breakdown between enhancers and containers and if it is necessary.

@JesseFarebro: This would be a good conversation to have. One of the big issues I'm facing right now is composing a grid with multiple plugins.

Opened https://github.com/GriddleGriddle/Griddle/issues/733 to continue this conversation.

[JesseFarebro]

@dahlbyk anything else that needs to be done here or can this be merged?

[dahlbyk]

Thanks for the update, @JesseFarebro; I think we're close.

I'm thinking we should also update the LocalPlugin button containers to no longer set onClick, since the enhancer handles that. Otherwise we'll fire the action twice, right? (A no-op, but still.)

[shorja]

Hey, I was working with @JesseFarebro but he won't be able to contribute to the project from now on, is there anything else I can do on this PR?

[shorja]

@dahlbyk @ryanlanciaux Now that I think about it, why should the pagination buttons even be concerned with calling Griddle pagination actions when the table is not using the local plugin, shouldn't that be the responsibility of the external data store passing in new pageProperties? Also, after some testing, this change is causing problems with Griddle.