GriddleGriddle / Griddle

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

Is it possible to get the data by 'griddleKey'? #761

Closed MichaelDeBoey closed 6 years ago

MichaelDeBoey commented 7 years ago

Griddle version

^1.10.0

Problem

So I'm using my own RowEnhancer, so I can implement my own onClick, like this:

const components: GriddleComponents = {
  RowEnhancer: (OriginalRow: GriddleComponent<GriddleRowProps>) => (props: GriddleRowProps) => (
    <OriginalRow
      {...props}
      onClick={() => {
        console.log(`Clicked Row ${props.griddleKey}`);
        console.log('props', props);
      }}
    />
  ),
};

When I click on the row, I get the following log:

Click Row 32
props {griddleKey: 32, columnIds: Array(3), className: "griddle-row", Cell: ƒ}

The griddleKey here would be useful to get al the row-data back I guess, but I can't seem to find out how this should be done. 😕

Can somebody help me please?

radziksh commented 7 years ago

Hi, @MichaelDeBoey, I did like this (I use typescript):

import { connect } from "react-redux";

export const rowDataSelector = (state, { griddleKey }) => {
    return state
        .get("data")
        .find(r => r.get("griddleKey") === griddleKey)
        .toJSON();
};

export class CustomGrid extends React.Component<Props, {}> {
...
const StandardRow = ({ Cell, griddleKey, columnIds, style, className, rowData }) => {
    return (
        <tr
            onClick={(e) => {
                // dirty hack before griddle will implement normal onRowClick
                (e as any).rowData = rowData;
                this.props.onRowClick(e);
            }}
            key={griddleKey}
            style={style}
            id={rowData.id}
            className={className + " " + styles.tableRow}
        >
            {columnIds && columnIds.map(c => (
                <Cell
                    key={`${c}-${griddleKey}`}
                    griddleKey={griddleKey}
                    columnId={c}
                    style={style}
                    className={className}
                />
            ))}
        </tr>
    );
};

const standardRowMapStateToProps = (state, props) => {
             return {
                 rowData: rowDataSelector(state, props),
             };
         };

const enhancedStandardRow = connect(standardRowMapStateToProps)(StandardRow)

...
return(){
   <Griddle 
      components={{
         Row: enhancedStandardRow
      }}
dahlbyk commented 7 years ago

You'll get better performance using the rowDataSelector that's exported from Griddle (via our Storybook). You can also simplify your enhancer using recompose/mapProps:

import { selectors, GriddleComponents } from 'griddle-react';
import { connect } from 'react-redux';
import mapProps from 'recompose/mapProps';

const EnhanceWithRowData = connect((state, props) => ({
  rowData: selectors.rowDataSelector(state, props)
}));
const components: GriddleComponents = {
  RowEnhancer: EnhanceWithRowData(
    mapProps(props => ({
      ...props,
      onClick: () => {
        console.log(`Clicked Row ${props.griddleKey}`);
        console.log('props', props);
      },
    }))
  ),
};
MichaelDeBoey commented 7 years ago

@dahlbyk Thanks for the help! 🙂

Unfortunately I didn't manage to make this work 😕 I always get the following error:

Uncaught TypeError: state.getIn is not a function
    at Object.rowDataSelector (dataSelectors.js:299)
    at actions.js:7
    at Object.dispatch (index.js:11)
    at dispatch (<anonymous>:2:1620)
    at showDocument (index.js:200)
    at onClick (index.js:65)
    at HTMLUnknownElement.callCallback (react-dom.development.js:540)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:579)
    at Object.invokeGuardedCallback (react-dom.development.js:436)
    at Object.invokeGuardedCallbackAndCatchFirstError (react-dom.development.js:450)
    at executeDispatch (react-dom.development.js:834)
    at executeDispatchesInOrder (react-dom.development.js:856)
    at executeDispatchesAndRelease (react-dom.development.js:954)
    at executeDispatchesAndReleaseTopLevel (react-dom.development.js:965)
    at Array.forEach (<anonymous>)
    at forEachAccumulated (react-dom.development.js:933)
    at processEventQueue (react-dom.development.js:1105)
    at runEventQueueInBatch (react-dom.development.js:3600)
    at handleTopLevel (react-dom.development.js:3609)
    at handleTopLevelImpl (react-dom.development.js:3340)
    at batchedUpdates (react-dom.development.js:11066)
    at batchedUpdates (react-dom.development.js:2323)
    at dispatchEvent (react-dom.development.js:3414)

I replecated this in a codesandbox, maybe you could point me in the right direction?

When I console.log the result of getState(), I get my own state, but can't access the Griddle state. 😕

MichaelDeBoey commented 7 years ago

@dahlbyk Doh The codesandbox is just a link to the default sandbox, will provide a new link tonight 🙂

MichaelDeBoey commented 6 years ago

@dahlbyk New codesandbox link: https://codesandbox.io/s/30qvo721x6 🙂

Just click on a row an you'll see the error I get 🙂 And you'll also see the state I get is just my own state 😕

CC: @ryanlanciaux @joellanciaux

dahlbyk commented 6 years ago

@MichaelDeBoey take a look at https://codesandbox.io/s/r50q23027o.

You're using rowDataSelector in a context that has access to your app store rather than Griddle's internal store; you need to use the selector inside a <Griddle /> and with utils.connect (or simply { connect } from 'griddle-react', as of #751).

I did mess up my example slightly; EnhanceWithRowData needs to be composed. It also occurs to me that withHandlers is a better way to create onClick:

const EnhanceWithRowData = utils.connect((state, props) => ({
  rowData: selectors.rowDataSelector(state, props)
}));

const components = {
  RowEnhancer: compose(
    EnhanceWithRowData,
    withHandlers({
      onClick: props => () => {
        console.log(`Clicked Row ${props.griddleKey}`);
        console.log('props', props);
      },
    })
  ),
};
MichaelDeBoey commented 6 years ago

@dahlbyk Thanks for the example. 🙂
But then I'll create a dependency to recompose (for the compose & withHandlers methods) and that's not preferred on the project I'm working on. 😕

I was planning on extracting the RowEnhancer into a separate component btw (like you can see in v2 of the sandbox), but that doesn't seem to work because the react-redux connect-method I use there 😕.

I want to make my components as clean as possible and want to do all the logic in the logRowData function itself, by getting the rowData from the state inside that function.
Don't know if that's possible in any way?

Replacing react-redux' connect with griddle-react's one (like suggested in #751 and like you can see in v3 of the sandbox) also doesn't work 😕

dahlbyk commented 6 years ago

But then I'll create a dependency to recompose (for the compose & withHandlers methods)

If you're using Griddle, you have a dependency on recompose. 😀

Replacing react-redux's connect with griddle-react's one (like suggested in #751 and like you can see in v3 of the sandbox) also doesn't work

Your sandbox is using Griddle 1.9, which doesn't include #751. Try this instead:

import Griddle, { utils, plugins } from 'griddle-react';
const { connect } = utils;

I want to make my components as clean as possible and want to do all the logic in the logRowData function itself, by getting the rowData from the state inside that function.

Ultimately you're going to keep running into the fact that there are two Redux stores, with two state objects, accessed from two connect functions. Try to separate Griddle concerns (mapping Griddle state to rowData in props) from your app's concerns (logging data).

Here's another demo that implements support for a generic onRowClick(griddleKey, rowData) event: https://codesandbox.io/s/pww5wrvqp7. This might give you more clear separation between the Griddle parts and the what-to-do-with-rowData parts? (Note that events is one of the few aspects of Griddle config that requires a plugin to override.)

MichaelDeBoey commented 6 years ago

@dahlbyk Thanks for your help. 🙂

If you're using Griddle, you have a dependency on recompose. 😀

I know, but I meant a direct dependency. 🙂

Your sandbox is using Griddle 1.9, which doesn't include #751

Ultimately you're going to keep running into the fact that there are two Redux stores, with two state objects, accessed from two connect functions.

The state passed to the mapStateToProps function is still my own state instead of the griddle-react state (like you can see in v4 of the sandbox)

I would think that if you use griddle-react's connect, it will pass the state of the grid and the getState passed to the logRowData function (via the thunk middleware), would also be griddle-react's state?

dahlbyk commented 6 years ago

The state passed to the mapStateToProps function is still my own state instead of the griddle-react state (like you can see in v4 of the sandbox)

I would think that if you use griddle-react's connect, it will pass the state of the grid and the getState passed to the logRowData function (via the thunk middleware), would also be griddle-react's state?

You're using Griddle's connect outside a <Griddle />, in which case it acts like normal connect.

Now also seems like a good time to point out that you can add Redux middleware to the Griddle store via <Griddle reduxMiddleware={[...] /> or a reduxMiddleware key on a plugin. So you have that option to enable redux-thunk for the Griddle store, in which case you could wire up dispatching your logRowData call as part of the RowEnhancer.

You have lots of options, just depends how exactly you want to break things up. 😀

MichaelDeBoey commented 6 years ago

OK so back to the separate RowEnhancer component then 🙂

Still breaks 'cause a TypeError (like you can see in v5 of the sandbox) 😕 Am I doing something wrong? I just made it a separate component and added the utils.connect around it. So now griddle-react's connect is inside a <Griddle /> component...

dahlbyk commented 6 years ago

That RowEnhancer is a HOC is tripping you up. You're trying, conceptually:

connect(...)(OriginalRow => props => <OriginalRow ... />)

You need to do this instead:

OriginalRow => connect(...)(props => <OriginalRow ... />)

https://codesandbox.io/s/mk6nxyl9j

Composing HOCs is pretty messy, thus using Recompose:

compose(
  connect(...),
  mapProps(...)
)
MichaelDeBoey commented 6 years ago

@dahlbyk OK thanks for the explanation, that's one step closer to the solution 🙂

I now get the following error (like you can see in v6 of the codesandbox):

Actions must be plain objects. Use custom middleware for async actions.

So I just added the thunk middleware to the reduxMiddleware attribute of the <Griddle /> component (like you can see in v7 of the codesandbox) and it works! 🎉🎉🎉🎉🎉

import thunk from 'redux-thunk';

// ...

return <Griddle reduxMiddleware={[thunk]} />;
dahlbyk commented 6 years ago

Great! I'm going to close this, but let us know if you have any other questions.

MichaelDeBoey commented 6 years ago

Thanks for all your help @dahlbyk! Really appreciate it! 🙂