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

Toolpad shows 2 unsaved changes when adding a component #781

Closed
2 tasks done
Janpot opened this issue Aug 12, 2022 · 5 comments · Fixed by #861
Closed
2 tasks done

Toolpad shows 2 unsaved changes when adding a component #781

Janpot opened this issue Aug 12, 2022 · 5 comments · Fixed by #861
Assignees
Labels
bug 🐛 Something doesn't work priority: low To delay as much as possible
Milestone

Comments

@Janpot
Copy link
Member

Janpot commented Aug 12, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

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

Screen.Recording.2022-08-12.at.08.48.19.mov

Expected behavior 🤔

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

Steps to reproduce 🕹

Steps:

Context 🔦

No response

Your environment 🌎

No response

@Janpot Janpot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer bug 🐛 Something doesn't work priority: low To delay as much as possible and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 12, 2022
@apedroferreira
Copy link
Member

apedroferreira commented Aug 16, 2022

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
Copy link
Member Author

Janpot commented Aug 16, 2022

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
Copy link
Member

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

@apedroferreira apedroferreira self-assigned this Aug 16, 2022
@prakhargupta1
Copy link
Member

prakhargupta1 commented Aug 17, 2022

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
Copy link
Member

Created this issue to fix the larger issue with number of updates #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work priority: low To delay as much as possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants