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

Added example using a controlled React component and hooks. #735 #1531

Merged
merged 3 commits into from
Dec 10, 2020

Conversation

zachsa
Copy link
Contributor

@zachsa zachsa commented Dec 10, 2020

Description

Added an example do the demos folder using React hooks and a controlled, functional component.

}

useEffect(() => {
const grid = GridStack.init()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you calling init() multiple times ? not the right thing. call once and save the var instead. Also removing all then making widgets doesn't seem right. the Vue example does only on what got added...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Easy to call init() just once.

Looking at the Vue example I see that there is a button that adds an additional widget. However in my case that wouldn't work, as the component receives a list of items from a server that I would like to render as widgets. When I update the server, the component is re-rendered with an updated list of items that I would like to make into widgets. That is why I am using grid.makeWidget(someRef.current)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you're reading a list from a server, I would call load() instead of makeWidget(). ha but wait the DOM elements are already all there ? then I would wrap the loop with a batchUpdate()/commit() so layout is only done once...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - works. Also, I see that NOT specifying the row in the JSX that there is no bug regarding errant order changes (the bug that I mentioned earlier)

Copy link
Contributor

@jedwards1211 jedwards1211 Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adumesny if you need to have live React components (which is probably the common case), using load() isn't a very good option in general. I guess you could render React components into the widgets using ReactDOM.createPortal after load() creates the elements, but then you would still need other code to hook up widgets added later.

ref={refs.current[item.id]}
key={item.id}
className={'grid-stack-item'}
gs-w="12"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think you want entire row widget

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention here is that as a user adds items to the array (i.e. updates the database), that these new items come through to the client and get added to the bottom of the grid stack as rows. Users can then re-adjust the layout to incorporate those rows.

Is there a better way of achieving this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see that I would need to recreate the grid from some kind of state, otherwise grid items will always be full rows on first component render

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can serialize the x,y,w,h of each grid item. adding them to the bottom and making when a full row is just weird. Look at view adding a single widget but you say yours come from backend as complete list. Which may or may not be what you want in general. I would suggest showing both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the example to include the old react example, re-written using functional components and also the controlled example

@adumesny
Copy link
Member

thanks, that looks reasonable. would it make sense to replace the other one instead with this (or rename it as it maybe primary use case ?

@adumesny adumesny merged commit cc6d102 into gridstack:develop Dec 10, 2020
@zachsa
Copy link
Contributor Author

zachsa commented Dec 10, 2020

I think both are useful as references. I'm not sure which would be considered the main example, but I would easily be able to find react code that I find useful from these two files as they are named.

Also happy to make any further adjustments if you have any preference or ideas.

Thank you for merging!

@adumesny
Copy link
Member

I didn't run it/try it, but adding a single widget should NOT call removeAll() + makeWidget() - only the newly added widget should have a makeWidget() call like the Vue example. That's really inefficient and not a good example.

@zachsa
Copy link
Contributor Author

zachsa commented Dec 11, 2020

I'm using that in my work - it seems to work okay for the use case. The reason is that I also need to remove widgets, but the removed widgets are not present in the array when the component renders.

It's definitely possible to keep track of widgets that have been added/removed, but calling removeAll() is vastly simpler and unless there is an obvious performance problem the tradeoff is definitely worth it.

A much better option would be to call grid.isWidget(el). Could gridstack keep a dictionary of added elements somewhere? the key could be the hash of the the html string, and then every element turned into a widget just needs a unique id.

I agree that this is not necessary for the example though, since no elements are ever removed. (but the tricky part is all about removing elements, not adding them).

Actually... I see there is a means of getting existing grid items and then diffing those to the new items. But that requires creating a document fragment and getting an ID attribute from a top level node for every element in the grid. How would this compare to just calling removeAll() and re-adding?

@adumesny
Copy link
Member

adumesny commented Dec 11, 2020

grid.isWidget(el) is same as checking el.gridstackNode != undefined, and we keep track of nodes (grid.engine.nodes) which have node.el HTML element back so if the item has been removed from DOM grid.engine.nodes[i].el.parentNode == null and you could call removeWidget() on it. You can also store an id with each widget, to look things up (we also internally store _id that is guarantee to be unique for lookup and layouts). But you still need to track newly added (what the Vue.js examples do).

yeah if you have no easy way to track newly added (removed can be done as shown) and want to call on a list of items, then yes remove all and re-create will work abeit not very efficiently and hopefully not leaking memory along the way :)

looking at the code, I think removeAll(true which is default) fails to remove the DOM elements actually. OOops

@jedwards1211
Copy link
Contributor

jedwards1211 commented Aug 5, 2021

I just got to where I can drag an external widget onto my grid with React, but it's kind of awkward.

GridStack has already added the new DOM element to the grid, but it's out of React's control (it's best if you can avoid having any DOM state outside of React's control). And when I add the new item to local state, my React component is going to render an element for it and call makeWidget on it -- so I have to removeWidget on the element GridStack added for the drop, lest two copies of the new element appear.

I also would like to make the cloned external widget being dragged a live React component (e.g. when passing a helper function to initDragIn). Maybe it could be done with ReactDOM.createPortal? But of course it wouldn't ever get my grid component's enclosing React context; only the new widget created by my grid component would get its enclosing React context.

In general the two-way binding nature of GridStack and the one-way dataflow philosophy of React are in conflict, and the compatibility is more tenuous than the docs let on. I wish GridStack had a mode where it gives me change events without doing any DOM manipulation at all, and leaves it up to me to accomplish the DOM manipulations.

@adumesny
Copy link
Member

adumesny commented Aug 7, 2021

| I wish GridStack had a mode where it gives me change events without doing any DOM manipulation at all, and leaves it up to me to accomplish the DOM manipulations.

@jedwards1211 please file this as a feature request with a good reason/example showing why it would be better for certain frameworks.

Also if you can improve the existing React example for others, that would be great.

@jedwards1211
Copy link
Contributor

@adumesny

adding a single widget should NOT call removeAll() + makeWidget()

The only options for handling changes to the items array in a controlled React component are

  • do removeAll() + makeWidget()
  • compare the new item array with the DOM elements that are currently in the grid and makeWidget() / removeWidget() as necessary to bring the grid up to date

removeAll() + makeWidget() is less performant but has the virtue of simplicity. It also takes care of the issue I was talking about in my previous comment where I have to removeWidget on a newly dropped item.

It's fairly idiomatic anyway since in React, for better or worse, re-rendering a list after adding or removing an item involves creating O(n) new virtual DOM elements, which React then compares to the current virtual DOM, another O(n) operation. If only one element changed it will only make one update to the actual DOM. The main exception to this is virtualization libraries like react-virtualized or react-window that only render elements within the current scroll viewport of a list into the DOM.

I had made my own code call removeWidget when a single item I rendered unmounts, but this approach ended up a failure -- by this time React has already removed the element from the DOM so removeWidget ignores it.

@v1talii-dev
Copy link
Contributor

v1talii-dev commented Jul 20, 2022

I just got to where I can drag an external widget onto my grid with React, but it's kind of awkward.

GridStack has already added the new DOM element to the grid, but it's out of React's control (it's best if you can avoid having any DOM state outside of React's control). And when I add the new item to local state, my React component is going to render an element for it and call makeWidget on it -- so I have to removeWidget on the element GridStack added for the drop, lest two copies of the new element appear.

I also would like to make the cloned external widget being dragged a live React component (e.g. when passing a helper function to initDragIn). Maybe it could be done with ReactDOM.createPortal? But of course it wouldn't ever get my grid component's enclosing React context; only the new widget created by my grid component would get its enclosing React context.

In general the two-way binding nature of GridStack and the one-way dataflow philosophy of React are in conflict, and the compatibility is more tenuous than the docs let on. I wish GridStack had a mode where it gives me change events without doing any DOM manipulation at all, and leaves it up to me to accomplish the DOM manipulations.

Faced the same problem. To synchronize data, there must be a single entry and exit point, in my case for vue, this is a variable in the v-model. I don't need to control the DOM via gridstack, but it's enough to make changes in the v-model. I would like to implement a mode for the grid that would disable all DOM manipulations for drag and drop events, resizing, and so on. Does anyone have any idea how to do this?

@adumesny
Copy link
Member

adumesny commented Jul 21, 2022

so I use gridstack at work (or used to) with an angular wrapper that has a 'fd-grid' and 'fd-griditem` component that uses Angular dynamic object creation to create the Angular based layout dynamically from some REST json. during drag from outside (toolbar of widgets) I use helper placeholder images and on drop I replace them with the real Angular component (that gets created on the fly, just like during de-serialization). Never had an issue with dom manipulation and needing events only. And no I don't use ngFor loop.

yeah there are many ways to do Angular/React/Vue wrappers. it's outside of this lib to include those wrapper (and I can't open source my version anyway) but it would be nice to have more example that do work out of the box for sure...

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

Successfully merging this pull request may close these issues.

4 participants