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

Grid stack not updating after widget is removed with removeWidget() #2234

Closed
olivergreendev opened this issue Mar 17, 2023 · 9 comments · Fixed by #2288
Closed

Grid stack not updating after widget is removed with removeWidget() #2234

olivergreendev opened this issue Mar 17, 2023 · 9 comments · Fixed by #2288

Comments

@olivergreendev
Copy link

Subject of the issue

I have followed the tutorial found at https://www.binarcode.com/blog/build-interactive-dashboards-with-vuejs-and-tailwind which has its own codesandbox setup (which also has the same bug), but essentially when I click on the icon to delete a widget, it gets spliced from the widgets array (good), and also removed from the grid successfully (good), however the grid-stack in the DOM doesn't update its gs-curent-row, and thus its max-height inline style that gets calculated based off the current row. The widget node is 100% being removed from the DOM, just the grid-stack isn't updating itself, and thus when I go to add a new widget, there is always some blank space in the grid that affects the position of the new widget. When I use the removeAll function, it works great as it removes all the widgets from the DOM and array, as well as updates the grid-stack so new widgets get added from x: 0, y: 0 correctly.

Your environment

gridstack.js version: ^7.2.3
browser: chrome - Version 111.0.5563.65 (Official Build) (64-bit)

Steps to reproduce

Using the attached codesandbox, click the 'Start Editing' button, hover over each widget and click the trash can icon to delete all of the widgets. Click the 'Add Widget' button, and you'll see a new 'Widget 1' card gets added a few rows down, since the grid-stack is still detecting all the blank space from the previous widgets above it. What should happen, is the grid-stack should now be cleared, such that the new Widget 1 card gets added to position x: 0, y: 0 - but that is not the case.

You can find the codesandbox that was setup in the tutorial here https://codesandbox.io/p/sandbox/editable-dashboard-prototype-with-vue-gridstack-and-tailwind-css-65hxp5?file=%2Fsrc%2Fcomponents%2FEditableDashboard.vue&selection=%5B%7B%22endColumn%22%3A13%2C%22endLineNumber%22%3A116%2C%22startColumn%22%3A13%2C%22startLineNumber%22%3A116%7D%5D

All I have done is follow this tutorial, and produced the same bug as found in the tutorial codesandbox.

Expected behavior

If I remove a widget, the grid-stack should update it's data, so that there is no blank space that affects the auto-calculated positioning of new widgets.

image

image

@fredericrous
Copy link
Contributor

the example only works if you move widgets but don't add/remove anything.
What you should do instead of manipulating the dom is to let gridstack do its magic and instanciate a new vue app in each widget.... or better yet, use https://vuejs.org/guide/built-ins/teleport.html

@olivergreendev
Copy link
Author

Hi Fred, thanks for getting back to me on this one. I don't quite follow, would you be able to point me a bit more in the right direction please? I am unsure why when I remove a widget, the grid isn't updating and allowing widgets to be created in the coordinates previous widgets had taken up, but no longer do? If it is because I am doing something wrong, please highlight what it is I am doing wrong, and how you would better do it (some example code would be really helpful when you have time of course, no rush on my part, just grateful for some support). Thanks again!

@aszcoding
Copy link

@olivergreendev, I believe something similar was happening to me when I was trying to remove widgets.

grid.removeWidget(widget) will remove the widget from the grid, but it will still be in your DOM.
You need to use widget.remove() as well in order to remove the widgets from the DOM so it recognizes there is available space for new widgets there.

@olivergreendev
Copy link
Author

@aszcoding Thank you for your suggestion! I get an error saying widget.remove() is not a function because .remove() is not part of the docs. Could you perhaps elaborate a little further on your implementation so I can try and figure out what else needs to be done please? When the widgets are removed using removeWidget(), I can see the nodes are being removed from the DOM, it's just the grid stack doesn't update its row for example.

@adumesny
Copy link
Member

adumesny commented Apr 15, 2023

@olivergreendev @aszcoding that article has issues (see my comments there) and I would suggest our example instead (which I beleive are more correct - will have to double check as I don't use Vue)

as for removeWidget(el) (which removed the DOM by default but vue should be handling that IMO) it should update the grid row count - please post a simple example (no Vue) showing the issue using our demos.

@vandelpavel
Copy link

@adumesny
@olivergreendev

Problem:

You are using numbers as Ids (or more in general Ids that start with numbers)

Solution:

Add a prefix every time you refer to them inside the code
(notice that in the examples they are using "w_")

Explanation:

I'm using vue too (precisely through quasar framework)
In order to properly remove a widget by using the "v-for" mode you have to:

  1. Remove the element from the array used by the v-for
  2. Remove the element by the DOM and let grid stack know that

To do that you need to also call the "grid.removeWidget" function: link here
I used to have numeric ids for my items since they would have made it easier to develop BUT when I called that function and passed a selector like #1 I got the next error:

caught (in promise) DOMException: Failed to execute 'querySelectorAll' on 'Document': '#1' is not a valid selector.

That's because Ids must not start with a number, read here for more info

In order to make that work add a prefix (Ex: "w_") in every occurrence ex:

<div class="grid-stack">
      <div
        v-for="item in items"
        :id="'w_' + item.id" //        <=====  Here
        :key="item.id"
        class="grid-stack-item"
        :gs-x="item.x"
        :gs-y="item.y"
        :gs-w="item.w"
        :gs-h="item.h"
        :gs-id="item.id"
      >
       // Rest of your code
      </div>
</div>
//             Here  vvvv
   grid.removeWidget("#w_"+idToDelete, false)

@adumesny
Copy link
Member

adumesny commented Apr 27, 2023

great info.... or call grid.removeWidget(document.getElementById(item.id), false) - in most of my code I pass the DOM element anyway since I have quicker way to access it than search for it. GS support many ways, but best for you to pass the element....

@vandelpavel
Copy link

Yes, that would do the job!

getElementById works with HTML5 ids, which can start with numbers, while querySelector uses CSS selectors, which can not start with numbers.
You can choose what suits your needs.

adumesny added a commit to adumesny/gridstack.js that referenced this issue Apr 29, 2023
* fix gridstack#2234
* `Utils.getElements('1')` (called by removeWidget() and others) now checks for digit 'selector' (becomes an id)
@adumesny
Copy link
Member

fixed in next release. don't forget to donate if you find this lib useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants