search

mui / toolpad

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

Toolpad shows 2 unsaved changes when adding a component #781

Closed Janpot closed 2 years ago

Janpot commented 2 years ago

Duplicates

Latest version

Current behavior 😯

When adding a component in the canvas, 2 changes are recorded.

https://user-images.githubusercontent.com/2109932/184302833-9507f4f8-aaed-40e6-b796-c716b989eaea.mov

Expected behavior 🤔

This used to be just one change, why is the dom now updating twice?

Steps to reproduce 🕹

Steps:

1. 2. 3. 4.

Context 🔦

No response

Your environment 🌎

No response

apedroferreira commented 2 years ago

I think this happens when I change one or multiple elements' columnSize layout prop for the horizontal resizing - in some scenarios these values have to be updated to readjust, sometimes for several elements. From what I remember it wasn't easy to make things work without having to update that property for some elements in some cases, but I can take another look and see if it's possible. The main reason that it works like this is because if there's 3 elements/columns in a row for example, the total sum of the columnSize prop of each element is 3, but if there's 4 elements it's 4, etc... And when a new element is added to a row, it needs to have a default columnSize, which for this system is always 1.

Edit: this might not be 100% correct, and I'm also normalizing column sizes which might not be a necessary thing, so anyway I'll take a look and think if I can avoid unnecessary updates.

Janpot commented 2 years ago

I just added this ticket because it's another one of those things that will definitely make a bad UX if we should implement a naive undo/redo implementation

apedroferreira commented 2 years ago

sounds good, i edited my comment a bit because this might be improvable, will just have to take a look with reducing the number of updates in mind

prakhargupta1 commented 2 years ago

From the UX perspective, we can skip showing the count of unsaved changes and rather show a visual saying (on hover) 'All changes saved successfully'.

apedroferreira commented 2 years ago

Created this issue to fix the larger issue with number of updates https://github.com/mui/mui-toolpad/issues/845 For this issue will only fix the issue in the UI - by not showing the count of DOM updates but probably logging to the console in dev, or a similar solution.