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

Add generic type in datagrid #10607

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ogustavo-pereira
Copy link
Contributor

Problem

Closes #10595

Describe the problem this PR solves

Solution

Since the datagrid is a React.forwardRef type component, I can't create a generic one easily, so I need help to solve and bring the best solution to this challenge.

I used the article below as a basis to solve the problem
https://www.totaltypescript.com/forwardref-with-generic-components

Variable names are for illustration purposes only, they can be better planned if the solution is to your liking.
Describe the solution this PR implements

How To Test

In the datagrid component itself, you can create a test component passing a generic and seeing if this generic is going to the rowClick

At first I focused only on rowClick, which is my main problem today, if you need other places we can add it

Describe the steps required to test the changes

image

const Test = () => {
    return (
        <Datagrid<{ id: string; name: string }>
            rowClick={(id, resource, record) =>
                console.log({ id, resource, record })
            }   
        >
            <div></div>
        </Datagrid>
    );
};

Additional Checks

  • The PR targets master for a bugfix or a documentation fix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • The PR includes one or several stories (if not possible, describe why)
  • The documentation is up to date

Also, please make sure to read the contributing guidelines.

Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment on lines +121 to +124
const fixedForwardRef = <T, P = {}>(
render: (props: P, ref: React.Ref<T>) => React.ReactNode
): ((props: P & React.RefAttributes<T>) => React.ReactNode) =>
React.forwardRef(render) as any;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to genericForwardRef (just like our genericMemo) and extract it in its own file directly under src? I have a feeling we're going to use it elsewhere

): ((props: P & React.RefAttributes<T>) => React.ReactNode) =>
React.forwardRef(render) as any;

const WDatagrid = <RecordType extends RaRecord = any>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDatagrid? I suggest DatagridComponent

@@ -1,6 +1,6 @@
import { Identifier, RaRecord } from 'ra-core';

export type RowClickFunction = <RecordType extends RaRecord = RaRecord>(
export type RowClickFunction<RecordType extends RaRecord = RaRecord> = (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this in SimpleListItem from SimpleList too. Can you make it accept a RecordType as well?

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

Successfully merging this pull request may close these issues.

Making Datagrid Generic
2 participants