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

[data grid] Add recipe using a Portal to render components from the DataGrid outside its context #2522

Closed
Tracked by #9328
flaviendelangle opened this issue Sep 2, 2021 · 19 comments · Fixed by #10121
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation recipe

Comments

@flaviendelangle
Copy link
Member

flaviendelangle commented Sep 2, 2021

Summary 💡

This RFC aims at deciding whether or not we want to allow users to use our components outside of the scope of the DataGrid, and if so how could we make it work.

We currently have one example in our internal Storybook with a GridPreferencesPanel used outside of the grid scope. But it is not working anymore due to the introduction of the useGridRootProps hook which is not accessible outside of the grid scope.

export const OutsideColumnsPanel = () => {


If we want to allow this pattern, how could we make the rootProps correctly accessible ? If we just export the provider and the user gives the input props to it, our component will have access to the unprocessed props (meaning it won't have the built in classnames for instance, or the DataGrid free version forced props).
And as we move away from the single ref with everything in-it, this problem will probably occur a lot more.

@flaviendelangle flaviendelangle added component: data grid This is the name of the generic UI component, not the React module! discussion labels Sep 2, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 2, 2021

Looking at the source:

export const OutsideColumnsPanel = () => {
const data = useData(500, 50);
const apiRef = useGridApiRef();
const [open, setOpen] = React.useState(false);
const handleStateChange = React.useCallback((state: GridState) => {
const preferencePanelState = gridPreferencePanelStateSelector(state);
const isColumnsTabOpen =
preferencePanelState.openedPanelValue === GridPreferencePanelsValue.columns;
setOpen(isColumnsTabOpen);
}, []);
return (
<div style={{ display: 'flex', flexDirection: 'row', flex: 1 }}>
<GridApiContext.Provider value={apiRef}>
<SidePanel open={open} />
<div className="grid-container">
<DataGridPro
{...data}
apiRef={apiRef}
onStateChange={handleStateChange}
components={{
Panel: CustomPanel2,
Header: GridToolbar,
}}
/>
</div>
</GridApiContext.Provider>
</div>
);
};

I can think of three options:

  1. Removing the useGridApiRef() hook and GridApiContext.Provider from the public API. This hook currently serves two different use cases for the public usrs. First for composed data grid, to get a ref on the state. Using a useRef should be enough. Second, to initiate the state when creating a custom data grid. I would advocate that we could create a wrapper root component that takes care of the initial setup, with the context providers, hooks etc.
  2. Asking developers to use the components slot, for instance, the toolbar or the footer slot.
  3. Making the root props provider public. Basically, make the APIs used in the DataGrid and DataGridPro components public
    <GridContextProvider apiRef={apiRef} props={props}>

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Sep 2, 2021

  1. Making the root props provider public. Basically, make the APIs used in the DataGrid and DataGridPro components public

On this point, how do you solve this ?

If we want to allow this pattern, how could we make the rootProps correctly accessible ? If we just export the provider and the user gives the input props to it, our component will have access to the unprocessed props (meaning it won't have the built in classnames for instance, or the DataGrid free version forced props).

@m4theushw
Copy link
Member

diff --git a/packages/storybook/src/stories/playground/customize-components.stories.tsx b/packages/storybook/src/stories/playground/customize-components.stories.tsx
index bfa51b08..ceadde52 100644
--- a/packages/storybook/src/stories/playground/customize-components.stories.tsx
+++ b/packages/storybook/src/stories/playground/customize-components.stories.tsx
@@ -410,7 +410,6 @@ const CustomPanel2 = (props) => {

 export const OutsideColumnsPanel = () => {
   const data = useData(500, 50);
-  const apiRef = useGridApiRef();
   const [open, setOpen] = React.useState(false);

   const handleStateChange = React.useCallback((state: GridState) => {
@@ -420,22 +419,31 @@ export const OutsideColumnsPanel = () => {
     setOpen(isColumnsTabOpen);
   }, []);

+  const props = useDataGridProProps({
+    ...data,
+    ...data,
+    onStateChange: handleStateChange,
+    components: {
+      Panel: CustomPanel2,
+      Header: GridToolbar,
+    },
+  });
+  const apiRef = useGridApiRef();
+  useDataGridProComponent(apiRef, props);
+
   return (
     <div style={{ display: 'flex', flexDirection: 'row', flex: 1 }}>
-      <GridApiContext.Provider value={apiRef}>
+      <GridContextProvider apiRef={apiRef} props={props}>
         <SidePanel open={open} />
         <div className="grid-container">
-          <DataGridPro
-            {...data}
-            apiRef={apiRef}
-            onStateChange={handleStateChange}
-            components={{
-              Panel: CustomPanel2,
-              Header: GridToolbar,
-            }}
-          />
+          <GridRoot>
+            <GridErrorHandler>
+              <GridHeaderPlaceholder />
+              <GridBody />
+              <GridFooterPlaceholder />
+            </GridErrorHandler>
+          </GridRoot>
         </div>
-      </GridApiContext.Provider>
+      </GridContextProvider>
     </div>
   );

@m4theushw
Copy link
Member

m4theushw commented Sep 2, 2021

If we want to allow this pattern, how could we make the rootProps correctly accessible?

We could have a component wrapping the <GridRoot> and its children and add another hook wrapping useDataGridProProps and useDataGridProComponent. That way we can make this example to work again.

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Sep 3, 2021

I agree that we can make the story work by copy pasting most of the code of DataGridPro.tsx
But if we have a better solution.

For your 2nd comment, are you thinking about something like this ?

const MyCustomDataGridPro = () => {
  const { props, apiRef } = useDataGridPro(inputProps);

  return (
    <GridContextProvider props={props} apiRef={apiRef}>
      <GridPreferencesPanel />
      <DataGridProContent />
    </GridContextProvider>
}

With

  • useDataGridPro being a hook of @mui/x-data-grid-pro that calls useDataGridProComponent and useDataGridProProps

  • DataGridProContent being a component that contains GridRoot and its children


Maybe we should only do this for the paid version if we don't want user to do this with the free version

const MyCustomDataGrid = () => {
  const { props, apiRef } = useDataGrid(inputProps);

  props.disableColumnReorder = false;

  return (
    <GridContextProvider props={props} apiRef={apiRef}>
      <DataGridContent />
    </GridContextProvider>
}

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 4, 2021

cc @DanailH for thoughts on this discussion around the product direction.


I would personally +1 for the option 3. Of #2522 (comment) and what @m4theushw suggested in #2522 (comment). If we are uncomfortable with making the API public at this point, we could prefix it as unstable.

The value I see in this option is making a step toward allowing the developers to have more control of what's rendered, by breaking the big component monolith. It gets us closer to the headless hook API of react-table.

It also resonates with a thread I opened around should we make the backbone of the apiRef in the MIT plan, so that the community can build around it? It would make our initiative more likely to take shares in react-table's audience, which might lead to more upsells.

@DanailH
Copy link
Member

DanailH commented Sep 7, 2021

I read all the comments. I guess we never asked or more like answered the question if we want to support that behaviour in the first place.

Previously we discussed the idea of supporting the React Table way (hooks all the way and ability for developers to compose their own grid) as one of the directions we can take with the DataGrid. I think we should first have a decision on that because that is the root of whatever follows next.

If we say we would like to have that one day then supporting the described behaviour in this ticket makes sense.

Regarding the suggested options here #2522 (comment) Maybe I'm missing something but if we expose the API what would be the definitive feature of the Pro version? We would need additional logic to strip specific APIs (which should already be partially possible by just removing the Pro feature hooks from here useDataGridComponent.ts).

@flaviendelangle
Copy link
Member Author

Previously we discussed the idea of supporting the React Table way (hooks all the way and ability for developers to compose their own grid) as one of the directions we can take with the DataGrid. I think we should first have a decision on that because that is the root of whatever follows next.

100% agree on that.

what would be the definitive feature of the Pro version

If we just expose the apiRef, the pro version would still be the only one with the advanced features of some shared hooks.
For instance multiple column filtering and sorting or multiple selection.
And we can still add new hooks only on the pro version (useGridColumnReorder for instance, or useGridTreeData in the future).

But I do agree that we should clarify what should be the specificity of the Pro version.

@m4theushw
Copy link
Member

For your 2nd comment, are you thinking about something like this ?

@flaviendelangle Yes, but I think that the license verification should be on this new hook too. Currently, the license is only verified if the Watermark component is added. If the user doesn't include it, then there's no watermark and he could use the grid without a license.

Maybe we should only do this for the paid version if we don't want user to do this with the free version

There're some ways to force the props and prevent developers from enabling Pro features.

  1. Call Object.freeze on the props returned by the hook.
  2. Force the props in GridContextProvider before passing them to GridRootPropsContext. Then, mount the feature hooks inside the root context and get the props from it.

https://github.com/mui-org/material-ui-x/blob/253400ffce4f2c00120c258df450f01c6e962664/packages/grid/data-grid/src/useDataGridComponent.tsx#L50

Maybe I'm missing something but if we expose the API what would be the definitive feature of the Pro version?

@DanailH In the Pro version, everything is tied together and we guarantee it will work. The "definitive feature" is not to have to understand the grid's internals to have a Pro functionality. However, I think that by exposing the API, some of the features could be easily recreated only by disabling the default behavior of a couple of events and copying the code from GitHub. Take as example the multiple selection, where it listens only one event, it's easy to hack and have this feature for free. To prevent this from happening, should we make the possibility to cancel the default behavior a Pro feature? The problem I see is that the component tree is the same between the two plans, so all events are already in the free version. With complex features yet-to-be-developed, we could have different structures and make it harder to copy features from the Pro version.

@flaviendelangle
Copy link
Member Author

@m4theushw

Yes, but I think that the license verification should be on this new hook too

That sounds logical indeed.

@designbyadrian
Copy link

I just tried to use useGridSlotComponentProps(); in a component outside the DataGrid component hoping there was magic keeping the context between them, but got the Material-UI X: Could not find the data grid context. error.

Exposing the apiRef and calling this hook with the ref would have been useful.

@flaviendelangle
Copy link
Member Author

Not everything is in the ref. And not everything can be in it if we want to take advantage of React lifecycle.

We are currently trying to find a way to cleanly call components outside of the Grid scope.

@m4theushw
Copy link
Member

Here's an example rendering the filter and columns panels outside the DataGrid using Portals: https://codesandbox.io/s/inspiring-monad-tee4qi?file=/demo.tsx

@m4theushw m4theushw moved this from 🆕 Needs refinement to 🔖 Ready in MUI X Data Grid Oct 25, 2022
@m4theushw m4theushw changed the title [RFC] Use the DataGrid custom components outside of the DataGrid [data grid] Add recipe using a Portal to render components from the DataGrid outside its context Oct 25, 2022
@DanailH DanailH moved this from 🔖 Ready to 🏗 In progress in MUI X Data Grid Apr 20, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 30, 2023

Having a receipt is great, but it uses a portal, this feels more like a workaround than a dedicated solution, e.g. no SSR possible.

I would maybe suggest to reopen and undo this issue title change:

m4theushw changed the title [RFC] Use the DataGrid custom components outside of the DataGrid [data grid] Add recipe using a Portal to render components from the DataGrid outside its context

But we can wait more user inputs.

@DanailH
Copy link
Member

DanailH commented Aug 31, 2023

We can undo and reopen. The main challenge is that implementing this is quite a challenge and it might already be planned for the future (post v7).
CC @cherniavskii @joserodolfofreitas

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 31, 2023

it might already be planned for the future

@DanailH is there an issue open with this plan? Referencing this issue in that one sounds like a great option as well. We could frame it around the struggle with the lack of SSR support, cascading rendering, and the unintuitive React tree structure of the current portal based receipt.

@DanailH
Copy link
Member

DanailH commented Aug 31, 2023

I would imagine that this would be closely tied to the "compose your own grid" end goal so I'm not sure if there is an existing issue but I'll check. I tagged @joserodolfofreitas, maybe he keeps track of this already somewhere.

@oliviertassinari
Copy link
Member

@DanailH I have created #10474 for this idea. We will see if developers find it valuable, who knows, they might not.

@omer-pon
Copy link

Here's an example rendering the filter and columns panels outside the DataGrid using Portals: https://codesandbox.io/s/inspiring-monad-tee4qi?file=/demo.tsx

Hi, thanks for the demo however in this demo filter is not outside of the DataGrid, nor the footer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation recipe
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants