How should the parent pass in new layout(s)?

[STRML]

Related to #98

This is an open issue for comment.

Currently, the following methods are used to determine whether or not to reload the layout:

RGL:

!_.isEqual(this.props.layout, nextProps.layout)

ResponsiveRGL:

``!_.isEqual(this.props.layouts, nextProps.layouts)`

This doesn't adequately cover some use issues, like:

• (Responsive Grid Layout reset doesn't do anything #98) - If I reset the layout, to what was originally passed in, the props haven't changed, so the layout doesn't reset.

This is actually very similar to the problem that caused me to rewrite <Draggable> in the first place. It's difficult, in React, to enforce a state reset on children at some times while allowing them to manage their own state at other times.

Should users just simply grab a ref and setState() themselves? I don't know if I see a better option.

[STRML]

Worse still, if you reset a layout by calling setState() on RRGL, it will only work once, because after that the new props always match the old props.

I don't want to move the check back to !isEqual(this.state.layout, nextProps.layout) because it seems that any errant rerender of the parent could cause the layout to reset. This means you'd have to always be certain to watch onLayoutChanged and setState() every time, which feels incredibly wasteful.

[menelike]

Would this be solved if every passed prop and every state would be immutable?
Currently a lot of changes to layouts are made in place, but this sometimes makes debugging frustrating and you'll always need to look if your components update as they should.
As a result (surely with a performance loss) I always convert layouts to immutable (Immutable.fromJS(layouts)) and later pass Responsive RGL the reconverted mutable back (layouts.toJS()).

I use RGL on a Flux/Redux Immutable js stack. Maybe in long term it would be better to switch to Redux as well.

[plandem]

+1 for redux

[STRML]

Well, we definitely wouldn't use it in a library - not sure what you're suggesting. And Immutable is very slow and a large dependency.

We could clone the layout items when they come in. Since the shapes are regular we could do a very fast custom clone. That should help and we could start implementing a better SCU.

[menelike]

@STRML redux without any explanation was indeed misleading. I may miss the point and this is total nonsense:

imho shouldComponentUpdate should only care about "do we need to render?" and never about "do we need to sync our states and then render?" Currently we have two sources of truth, props and states and we need to determine which to trust or if we merge. If we leave out the props we cut the API and if we leave out the states we loose complete control over the grid.

We could create a simple object which stores the layout and has a function to every action like 'item drag start', 'item dragged', 'item drag stop' which each call a list of reducer functions (move, compact etc.) to mutate the layout. With this solution reducers can also be overridden or enhanced with custom user defined functions. This would also move all layout logic from RGL to the store which leads to a clean and solid architecture. Testing and middleware functions should profit as well.

Moreover the store could be passed as a simple provider HOC where layouts are passed down as props and action functions are made available as context. The current RGL and Responsive RGL components would then only deal with view logic and action calls.

On the downside: setState will always be faster since we don't go trough the whole component chain. This makes a significant difference on drag events for example. You can't avoid component states in every case.

Concerning immutable: imho the first reason for immutable is cloning, the second is comparing.
If there are better ways of comparing nested objects immutable should be considered optional.

[STRML]

I'll have to bench it, but my intuition is that given a fast cloning function that assumes an object shape, we'll be able to clone layouts faster than we would with Immutable, or at least fast enough to make the large dependency not worthwhile. If the cloning can be properly optimized, it can be very, very fast. Normal _.clone()-style functions are not because they have to do property lookups, which causes the underlying object to become a slow object / hash, which is why, confusingly, even JSON.parse(JSON.stringify(object)) can be faster.

In a sense, I think the proper API may be:

<ReactGridLayout initialLayout={layout} ref="grid"...>

This makes it clear that this property, if changed, will be ignored.

And when you need to force-push a new layout, you do this.refs.grid.resetLayout(newLayout).

I don't see a better way to do such an imperative action with React's declarative nature.

I agree re: reducer functions, which is essentially what we already have in utils, except they're mutating data rather than cloning it. We should absolutely expose these in the package so they can be overridden. This might be difficult with Babel's import/export syntax, I'm fine with moving back to module.exports (less gotchas anyway).

I'm not sure we need to basically re-implement Redux here otherwise. That will make the API of this component significantly different than most React components, and I don't see a good reason for that. There's a lot of low-hanging fruit here in terms of shouldComponentUpdate on <GridItem> and fast clones. Once we do that work we can see where we are and whether or not more drastic steps are needed.

[menelike]

In my own project I needed to know if stateless components with explicit shouldComponentUpdate could deal with high frequency event updates. I took RGL to build a POC, code at https://github.com/menelike/react-grid-layout/tree/performance
Test were only made with 0-showcase, this code might not work on several different cases.

Principles:

1. Updates should be only forced trough trough shouldComponentUpdate

shouldComponentUpdate(nextProps) {
    if (this.props.placeholder !== nextProps.placeholder) return true;
    if (!this.props.children.length !== nextProps.children.length) return true;
    if (!this.props.layout !== nextProps.layout) return true;
    if (!this.props.containerWidth !== nextProps.containerWidth) return true;
    return false;
  }

Therefore changed layouts need to be cloned in order to force an update.
2. Only Props are needed to determine if an update is needed (no state, no context)
3. Within shouldComponentUpdate we only check for equality in ReactGridLayout and because it's more convenient 'shallowEqual' in GridItem

 shouldComponentUpdate(nextProps, nextState) {
    return (!shallowEqual(this.props, nextProps));
  }

Simplified design:

1. Add LayoutProvider as HOC where every grid changing logic takes place.
   This is needed to separate logic Performance tasks from the View, therefore a lot of additional tweaks has been done in here as well.
2. Make ReactGridLayout stateless.
   Same as 1 but vice versa, making ReactGridLayout a dumb component with low complexity.
3. Move props from ReactGridLayout to context.
   This makes it easier to debug unwanted renderings. We always know, that no context can conflict with performance which gives you a lot peace of mind for future changes.

Results:

I still do not know to make simple synthetic tests in this case. I made my test on OSX/lastest stable Chrome with 0-showcase example and optimized build configs (taken relevant parts from webpack.config.js to webpack-dev-server.config.js)

I made several runs, here are two graphs to each branch:
release 0.10.6:

https://github.com/menelike/react-grid-layout/commits/performance

Scripting performance is reduced, painting increased, overall reduced. But I'm not an expert on this, further tests are required, results could be absolutely misleading.

[plandem]

i created something like prototype for redux wrapper for grid:
https://github.com/plandem/redux-react-grid-layout

i'm still new at react/redux so sure it's not a best solution. It's not a real package yet, still working on it - just wanted to put it online, because i already have some question:

looks like update layout is not enough, because Grid does not update state :( Drag/Resize works fine because i update layout with 'layout' that came from callback. But when i remove item, it does not work - redux shows that state was updates, but grid is not updated visually (i mean item was removed, but compact was not happen) :( Any ideas?

[menelike]

@plandem without investigating your code. Set all shouldComponentUpdate to return true, if this solves the problem check what exactly was responsible for returning false.

[plandem]

i will try, i hoped that common 'connect' from redux will be enough, but if i have to add custom logic to 'shouldComponentUpdate' then looks like i will have to create HOC manually :(

i talked about this HOC that came from connect:
https://github.com/plandem/redux-react-grid-layout/blob/master/grid.js

[plandem]

'shouldComponentUpdate' does not help

[menelike]

@plandem my code mentioned above could work better with higher redux stores. But keep in mind that

1. layout object is mutable and by newLayout.fromJS(layout) in a reducer you would create a new layout and therefore and update will occur.
2. If you do changes on the (mutable) layout, return const newLayout = layout.splice(0) so that layout !== newLayout returns true

[plandem]

at my reducer i do:

            case types.REMOVE:
                return Object.assign({}, state, { layout: compactor(removeItem(state, action.id)) });

where:

function removeItem(state, id) {
    return state.layout.filter(item => (item.i !== id) )
}

and compactor is:

function compact(layout) {
......
    return layout.map((item, i) => updated[i] ? { ...item } : item);
}

so as you see - after 'remove' action reducer update layout totally. And it's updated at store if i debug, but at grid - nope :(

const ReactGrid = WidthProvider(ReactGridLayout);
...
render() {
        return (
            <ReactGrid className="layout"
                       layout={this.props.layout}
....

although grid is using layout prop, so it must actually get updates :(

[plandem]

actually i would like to have stateless Grid and HOC for layout changes. In that case custom logic can be attached more easily, for example all these -'drag/resize' events with redux must be used at reducers. But in current scenario, looks like it's not possible :( So we can't separate 'view' and logic for view :(

[menelike]

@plandem I may miss the point, but my code is made for exactly that use case. If you store the layout which has been sent to your higher function trough onLayoutChange you can do whatever you want, just make sure that you pass in a new layout with a new object reference (e.g. layout.splice(0)).

I haven't tested this with outer Redux/Immutable stack where you only store immutable objects and therefore converting layout to immutable would cause an update, even if they layout hasn't changed at all. This is because we would need deep comparisons here.
I had the idea to handle differently on immutable layout changes, so when an immutable layout object is passed a deep comparison will follow. Still working on how to deal with this special scenario.

[plandem]

yeah, your code looks like right way. but it's fork in that case :( i was talking about structure of that library :)

for optimisation probably this can help:
http://redux.js.org/docs/recipes/ComputingDerivedData.html
https://github.com/reactjs/reselect

[menelike]

that's what I tried first, but I then realized the component needs to be stateless and the impact on existing code can't be done with just a HOC (we do not need a HOC anway, gonna change that soon). I actually need to do further tests, I'm not sure, but this code could actually replace existing code without an impact on the API

imho the current master solves the same issue, just not in the stateless way. I still need to understand #164 for a final conclusion.

concerning reselect, that's a very good choice in an application but not in a library. @STRML was absolutely right when he said redux/immutable should not be used in a library. Therefore reselect should provide your application component and not in RGL. There you can also use your own shouldComponentUpdate to avoid unneeded updates.

[bkeating]

I just ran into this today before heading home. Hope to report back with findings and ideas. Love this package.

[preethijayanthi]

when minimizing the screen the grids are not minimizing can anyone please solve it

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 7 days