[DataGrid] Ignore drag events when disableColumnReorder is true #1952
Conversation
oliviertassinari
added
bug 🐛
There was a problem hiding this comment.
We could consider writing an e2e test case for the reoder with: https://stackoverflow.com/a/64746679/2801714. It's not directly related to the two different bugs we fix, but 🤷♂️
|
Looks nice but you would also need to make ``
I'm also in favour of adding e2e test, it can be a simple one. |
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Done. |
| @@ -42,6 +43,7 @@ export const useGridColumnReorder = (apiRef: GridApiRef): void => { | |||
|
|
|||
| const [, setGridState, forceUpdate] = useGridState(apiRef); | |||
| const dragCol = useGridSelector(apiRef, gridColumnReorderDragColSelector); | |||
| const options = useGridSelector(apiRef, optionsSelector); | |||
There was a problem hiding this comment.
you can destructure 'disableColumnReorder'
There was a problem hiding this comment.
Actually, in the future, if we could refactor the other usages of useGridSelector(apiRef, optionsSelector) to not destructure, it could be great. It would make it easier to find how the top level props are used in the codebase, allowing to grep option.x instead of x. These options tend to be all over the placef
| @@ -42,6 +43,7 @@ export const useGridColumnReorder = (apiRef: GridApiRef): void => { | |||
|
|
|||
| const [, setGridState, forceUpdate] = useGridState(apiRef); | |||
| const dragCol = useGridSelector(apiRef, gridColumnReorderDragColSelector); | |||
| const options = useGridSelector(apiRef, optionsSelector); | |||
There was a problem hiding this comment.
Actually, in the future, if we could refactor the other usages of useGridSelector(apiRef, optionsSelector) to not destructure, it could be great. It would make it easier to find how the top level props are used in the codebase, allowing to grep option.x instead of x. These options tend to be all over the placef
| await page.mouse.move( | ||
| brandBoundingBox.x + brandBoundingBox.width / 2, | ||
| brandBoundingBox.y + brandBoundingBox.height / 2, | ||
| { steps: 5 }, | ||
| ); |
There was a problem hiding this comment.
Looking at the logic, we could almost consider a drag & drop abstraction. Two repetitions might be a bit short to justify it. I'm only saying that it starts to show potential.
There was a problem hiding this comment.
Not just abstract the drag & drop but also other page.evaluate in this file.
There was a problem hiding this comment.
We could have a chat with Sebastian about how to abstract the common patterns. We might be close to have enough tests here and in the core to make good decisions.
Fixes #1579
Fixes #1762
Preview https://deploy-preview-1952--material-ui-x.netlify.app/components/data-grid/demo/