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

[XGrid] ViewPort doesn't update on table state change when using the useApiRef in React.StrictMode #849

Closed
MTPrescience opened this issue Jan 13, 2021 · 3 comments · Fixed by #933
Labels
new feature New feature or request
Milestone

Comments

@MTPrescience
Copy link

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

Current Behavior 😯

When using the useApiRef in React strictMode - the XGrid stops re-rendering view - but table state updates.
The re-render can then be triggered by resizing the window.

Expected Behavior 🤔

Grid should keep current functionality and provide extra flexibility through the api.

Steps to Reproduce 🕹

https://codesandbox.io/s/sad-tu-tn5zs

Steps:

  1. Click e.g sort.
  2. Nothing happens.
  3. Resize the window.
  4. Sorting applies

Context 🔦

Your Environment 🌎

`npx @material-ui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @material-ui/envinfo` goes here.
@MTPrescience MTPrescience added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 13, 2021
@MTPrescience MTPrescience changed the title Grid ViewPort doesn't update on table state change when using the useApiRef in React.StrictMode [XGrid] ViewPort doesn't update on table state change when using the useApiRef in React.StrictMode Jan 14, 2021
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work components: XGrid and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 14, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 14, 2021

@MTPrescience Thanks for reporting the issue. This issue happens because we aren't yet able to work with strict mode enabled. Material-UI v4 is not really compatible with it (partially). This should be possible once we upgrade to Material-UI v5.

You might have reported the same issue that we have ignored (strict: false) in the tests (so far), for instance:

https://github.com/mui-org/material-ui-x/blob/6e87cb34f54836a4eb18e71363817c40cce39953/packages/grid/x-grid/src/tests/filtering.XGrid.test.tsx#L82-L100

Out of curiosity what's your use case for using the apiRef?

@dtassone Did you try to leverage? storybookjs/storybook#12734 We can't enable strict mode in the documentation until v5 because it breaks some of the styles but maybe it works in Storybook with v4.


A side note. The codesandbox is a good reminder that we should treat modules in Material-UI as a global namespace. We need to prefix the exported data grid modules with Grid.

import { XGrid } from "@material-ui/x-grid";
import { Grid } from "@material-ui/core";

@MTPrescience
Copy link
Author

@oliviertassinari - Thanks for your reply. Let's put that information together with the little information about the apiRef on your website then. Most people are not reading your tests before deciding on using a component ;-)

I have several use cases atm.

  1. We are able to open modals off the grid and edit one row of the grid from that modal. We are using the updateRows of your api.

  2. We have implemented our own solution to in-cell editing until your version is up and running - so similar to above.

  3. We have user settings for each user for filters, column widths, column order etc. saved on the backend so we are using the api to listen for different events.

  • And we probably have even more cases in a day or two as we progress into this.

@oliviertassinari oliviertassinari added new feature New feature or request and removed bug 🐛 Something doesn't work labels Jan 18, 2021
@oliviertassinari oliviertassinari added this to the Sprint 9 milestone Jan 18, 2021
@dtassone
Copy link
Member

dtassone commented Jan 20, 2021

I had a quick look and it works if you don't use the apiRef prop, I will investigate further
Check out
https://codesandbox.io/s/morning-sea-11tlk?file=/src/App.js

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

Successfully merging a pull request may close this issue.

3 participants