Only update dashboard positions when necessary #4530
Conversation
Clicking anywhere on an unlocked widget generates a drag event, which in our case ends up sending a request to the server to update widgets positions. Use React Grid Layout `draggableCancel` prop, to avoid calling drag callbacks when a user clicks on the edit or delete buttons for a widget.
Optimize the number of times we compute and update the layout of a ReactGridContainer component: - Compute ReactGridContainer when the `positions` prop changes - Compare layout changes with the current computed layout to only call `onPositionsChange` when the layout did actually change This will reduce the number of times the layout of a ReactGridContainer component is computed and, more importantly, the number of update positions requests we send to the server.
ghost
assigned 
|
Oh, I forgot: I tried to write some tests to check how well the changes worked but it end up being too complicated. To test the functionality we need that react-grid-layout works but with enzyme it can only go to a point. I gave up after I needed to stub a couple of |
| // `onLayoutChange` may be triggered when clicking somewhere in a widget, check before propagating the change. | ||
| // Filter out additional Object properties in nextLayout, as it comes directly from react-grid-layout | ||
| const filteredNewLayout = newLayout.map(item => ({ i: item.i, x: item.x, y: item.y, h: item.h, w: item.w })); | ||
| if (lodash.isEqual(this.state.layout, filteredNewLayout)) { |
There was a problem hiding this comment.
Would it make sense to move this kind of check into shouldComponentUpdate and keep the component stateless?
There was a problem hiding this comment.
I don't think so, as this callback is used by react-grid-layout to notify when a widget is dragged or resized and it's not modifying the state or deciding if the component should re-render. The check it's avoiding to call a prop when the layout didn't change.
|
@dennisoelkers do you have any other comments on this? :) |
|
@dennisoelkers did you find any time to look at this? I'm afraid more changes will come soon in these components and we will have to solve conflicts in them. |
Clicking anywhere on an unlocked widget generates a drag event, which in our case ends up sending a request to the server to update widgets positions. Use React Grid Layout `draggableCancel` prop, to avoid calling drag callbacks when a user clicks on the edit or delete buttons for a widget. Partial backport of 9488608 (#4530). Mitigates #4525
Clicking anywhere on an unlocked widget generates a drag event, which in our case ends up sending a request to the server to update widgets positions. Use React Grid Layout `draggableCancel` prop, to avoid calling drag callbacks when a user clicks on the edit or delete buttons for a widget. Partial backport of 9488608 (#4530). Mitigates #4525
This PR makes some modifications in the web interface to be smarter calculating and updating dashboard positions:
onLayoutChangecallback, avoiding doing unnecessary updates in the serverIn order to fix the issue in 2.4, I think we should only backport change number 1, avoiding to introduce any other potential issues in the stable release. I can take care of that once this PR is reviewed.
Before a solution for #4527 lands, these changes fix #4525.