mui / toolpad

Toolpad: Full stack components and low-code builder for dashboards and internal apps.
https://mui.com/toolpad/
MIT License
1.27k stars 282 forks source link

Resizing columns drops all previous changes to columns #1486

Closed bharatkashyap closed 1 year ago

bharatkashyap commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

Steps:

  1. Go to https://master--toolpad.mui.com/_toolpad/app/cl91ik41i0454axrr578xjjrp/pages/eo03kwx
  2. Delete a column in the DataGrid
  3. Resize any other column

Current behavior 😯

The deleted column reappears

There is another case where all columns disappear but I can not deterministically recreate that

Expected behavior 🤔

No other changes occur except the resizing

Context 🔦

No response

Your environment 🌎

No response

prakhargupta1 commented 1 year ago

The same thing also happens when you hide a column and then resize.

apedroferreira commented 1 year ago

Argh, I can look into this. Oh wait, I thought this was about resizing in the drag and drop... I'll unassign myself for now but can still take it if I see there's nothing else with higher priority.

Janpot commented 1 year ago

This should fix it:

diff --git a/packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderPanel.tsx b/packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderPanel.tsx
index 38f99b19..7c160a45 100644
--- a/packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderPanel.tsx
+++ b/packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderPanel.tsx
@@ -43,22 +43,25 @@ export default function RenderPanel({ className }: RenderPanelProps) {

   const handleInit = useEvent((initializedBridge: ToolpadBridge) => {
     initializedBridge.canvasEvents.on('propUpdated', (event) => {
-      const node = appDom.getNode(dom, event.nodeId as NodeId, 'element');
+      domApi.update((draft) => {
+        const node = appDom.getNode(draft, event.nodeId as NodeId, 'element');
         const actual = node.props?.[event.prop];
+
         if (actual && actual.type !== 'const') {
           console.warn(`Can't update a non-const prop "${event.prop}" on node "${node.id}"`);
-        return;
+          return draft;
         }

         const newValue: unknown =
           typeof event.value === 'function' ? event.value(actual?.value) : event.value;

-      domApi.update((draft) =>
-        appDom.setNodeNamespacedProp(draft, node, 'props', event.prop, {
+        draft = appDom.setNodeNamespacedProp(draft, node, 'props', event.prop, {
           type: 'const',
           value: newValue,
-        }),
-      );
+        });
+
+        return draft;
+      });
     });

     initializedBridge.canvasEvents.on('pageStateUpdated', (event) => {

Needs a test before we can merge