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

[DataGrid] Inject extra props to customized components #641

Closed
2 tasks done
pat154 opened this issue Nov 26, 2020 · 4 comments
Closed
2 tasks done

[DataGrid] Inject extra props to customized components #641

pat154 opened this issue Nov 26, 2020 · 4 comments
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@pat154
Copy link

pat154 commented Nov 26, 2020

Firstly, big thanks for such a great library! This is my first time using it and it's a pleasure to use. I will most likely be moving to XGrid too as I have my eye on some roadmap features!

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The Data Grid's components.header prop requires an argument of type React.FC, as opposed to a ReactElement.

<DataGrid
  rows={orders}
  columns={orderColumns}
  components={{
    header: () => <Header />
  }}
/>

This means that providing sharing state between the parent and the child always triggers a re-render. In my case, I have an input element and each time its value changes it loses focus.

Here is a basic example of what I am doing:

const Table = () => {
  const filterRows = (rows) => {
    return rows;
  };
  const [searchTerm, setSearchTerm] = useState("");
  const Header = () => (
    <input
      value={searchTerm}
      onChange={(event) => setSearchTerm(event.target.value)}
    />
  );
  return (
    <div style={{ height: 500 }}>
      <DataGrid
        rows={searchTerm ? filterRows(orders) : orders}
        columns={orderColumns}
        components={{
          header: Header
        }}
      />
    </div>
  );
};

Expected Behaviour 🤔

I expect to be able to do something like provide a ReactElement as a component that would mean I can have a controlled input in the header that works as expected whilst still able to access shared state.

Steps to Reproduce 🕹

https://codesandbox.io/s/compassionate-keldysh-z995k?file=/src/App.js

Steps:

  1. Enter something into the input field
  2. Notice how the input loses focus

Context 🔦

I'm trying to provide a search field in the header of the Data Grid to filter rows based on the string entered. Losing focus with each character entered results in an unacceptable user experience

Your Environment 🌎

Tech Version
@material-ui/core ^4.11.1
@material-ui/data-grid 4.0.0
React 17.0.1
Browser Chrome
TypeScript 4.1.2
@oliviertassinari oliviertassinari transferred this issue from mui/material-ui Nov 26, 2020
@dtassone
Copy link
Member

You create a new Header component at every render so it rerender. You should put it outside the Table component or use React.useMemo

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 27, 2020

@pat154 We have discussed this problem in mui/material-ui#21453, we are working on applying the conclusion to the whole codebase, we have migrated the Slider so far. In practice, it means that you will be able to do:

<XGrid components={{ Header }} componentsProps={{ header: { searchTerm } }} />

The current API forces you to rely on the context: mui/material-ui#21453 (comment) as creating a new component at each keystroke inside a memo isn't viable (e.g loss of focus).

@oliviertassinari oliviertassinari added new feature New feature or request breaking change component: data grid This is the name of the generic UI component, not the React module! labels Nov 27, 2020
@pat154
Copy link
Author

pat154 commented Nov 30, 2020

You create a new Header component at every render so it rerender. You should put it outside the Table component or use React.useMemo

I think the virtual DOM is smart enough to handle this. I believe it's the re-render at the components.header level causing the issue.

@pat154 We have discussed this problem in mui-org/material-ui#21453, we are working on applying the conclusion to the whole codebase, we have migrated the Slider so far. In practice, it means that you will be able to do:

<XGrid components={{ Header }} componentsProps={{ header: { searchTerm } }} />

The current API forces you to rely on the context: mui-org/material-ui#21453 (comment) as creating a new component at each keystroke inside a memo isn't viable (e.g loss of focus).

Thanks for this. Yeah I figured context might be a potential solution. I'll give it a try.

@oliviertassinari oliviertassinari changed the title Can't share Data Grid's components states without triggering re-renders [DataGrid] Inject extra props to customized components Nov 30, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 2, 2021

Solved in #851. @dtassone I told you we could have made a dedicated PR only for the componentsProps. It would have help developers their own. One chunk of value at the time :p. Considering the pattern is generic, the target is to apply it to all components in v5, I'm closing. I don't think that we will need to document it specifically to the DataGrid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants