[DataGrid] Improve the handling of events (rm capture, add event, add new props) #1158

dtassone · 2021-03-03T18:24:55Z

Breaking Changes

[DataGrid] Rename event handlers for consistency

-<DataGrid onCellHover
-<DataGrid onRowHover
+<DataGrid onCellOver
+<DataGrid onRowOver

Fix #1157
Fix #1108
Fix #273
Related to #947

TODO:

update the api docs
cell events (click, double click, over, out, enter, leave)
row events (click, double click, over, out, enter, leave)
column header events (click, double click, over, out, enter, leave)
add tests

oliviertassinari

I like the idea of using the synthetic event system of React rather than a native event. This should help/solve #891.

packages/grid/_modules_/grid/hooks/root/useEvents.ts

dtassone · 2021-03-05T16:00:09Z

Should we bind the events if there is a renderCell? How about with a renderEditCell?
We could leave that up to the customization?

oliviertassinari · 2021-03-06T17:46:26Z

Should we bind the events if there is a renderCell? How about with a renderEditCell?
We could leave that up to the customization?

If we keep renderCell and renderEditCell customizing the children of a cell, not the Cell itself, I think that we should keep the binding of the events in the internal of the data grid. So no.

By removing the event capture, we make it possible for developers to hook into event.nativeEvent inside the cells (not the whole Cell but the children).
If we move the event listener to the synthetic event system of React, we would remove the need to access .nativeEvent.

As a side note, it might be interesting to add a test case where we click in a button on a cell, add something in the event, and see this something available in the onRowClick/onCellClick listeners. It gets us one step closer to solving #891

packages/grid/_modules_/grid/models/api/gridCellApi.ts

packages/grid/_modules_/grid/models/gridOptions.tsx

packages/grid/_modules_/grid/components/GridCell.tsx

packages/grid/_modules_/grid/utils/domUtils.ts

dtassone · 2021-03-08T18:19:15Z

Issue with the event serialisation on commodity dataset

dtassone · 2021-03-09T18:10:34Z

I did some improvements on the storybook.
Check the story below, and the control tab to change the number of rows...
https://deploy-preview-1158--material-ui-x.netlify.app/storybook/?path=/story/x-grid-demos-playground--demo

oliviertassinari

You can use the documentation that Sebastian has added to debug the failing visual repression tests: https://github.com/mui-org/material-ui/blob/next/test/README.md#run-the-visual-regression-tests

packages/grid/_modules_/grid/models/gridOptions.tsx

oliviertassinari

We need to update the API pages. Hopefully, this pain with incentivize us to work on its automatic generation.

docs/pages/api-docs/x-grid.md

packages/grid/x-grid/src/tests/events.XGrid.test.tsx

oliviertassinari

Nice a large refactoring 😁

packages/grid/_modules_/grid/models/api/gridParamsApi.ts

packages/grid/x-grid/src/tests/events.XGrid.test.tsx

packages/grid/_modules_/grid/utils/domUtils.ts

packages/storybook/src/stories/grid-columns.stories.tsx

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

oliviertassinari

Great

done

tusharchutani · 2021-03-25T17:35:06Z

Just curious when will this be released?

dtassone · 2021-03-26T11:03:04Z

Just curious when will this be released?

v4.0.0-alpha-23 should be released already

dtassone requested a review from oliviertassinari March 3, 2021 18:25

dtassone mentioned this pull request Mar 3, 2021

[DataGrid] Don't listen in the capture phase #1156

Closed

dtassone marked this pull request as draft March 3, 2021 18:27

oliviertassinari reviewed Mar 3, 2021

View reviewed changes

dtassone closed this Mar 3, 2021

dtassone reopened this Mar 3, 2021

DanailH reviewed Mar 4, 2021

View reviewed changes

packages/grid/_modules_/grid/hooks/root/useEvents.ts Outdated Show resolved Hide resolved

oliviertassinari changed the title ~~[DataGrid] remove event capture~~ [DataGrid] Remove event capture Mar 5, 2021

oliviertassinari added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Mar 6, 2021

dtassone force-pushed the eventCapture branch from cb5baf2 to bf63ced Compare March 8, 2021 15:49

dtassone requested review from oliviertassinari and DanailH March 8, 2021 15:49

dtassone marked this pull request as ready for review March 8, 2021 15:50

dtassone commented Mar 8, 2021

View reviewed changes

packages/grid/_modules_/grid/models/api/gridCellApi.ts Outdated Show resolved Hide resolved

dtassone changed the title ~~[DataGrid] Remove event capture~~ [DataGrid] Refactor events, rm capture, allow stop propagation, add enter leave out Mar 8, 2021

oliviertassinari added the breaking change label Mar 8, 2021

oliviertassinari requested changes Mar 8, 2021

View reviewed changes

oliviertassinari changed the title ~~[DataGrid] Refactor events, rm capture, allow stop propagation, add enter leave out~~ [DataGrid] Improve the handling of DOM events (rm capture, expose event, add new ones) Mar 8, 2021

oliviertassinari changed the title ~~[DataGrid] Improve the handling of DOM events (rm capture, expose event, add new ones)~~ [DataGrid] Improve the handling of events (rm capture, add event, add new props) Mar 8, 2021

dtassone requested a review from oliviertassinari March 10, 2021 11:18

oliviertassinari reviewed Mar 10, 2021

View reviewed changes

packages/grid/_modules_/grid/models/gridOptions.tsx Show resolved Hide resolved

packages/grid/_modules_/grid/models/gridOptions.tsx Show resolved Hide resolved

dtassone added 4 commits March 10, 2021 12:52

remove event capture

bc6b9f7

refactor event system

576aed0

fix typo

cd5059d

Cleanup

0575403

oliviertassinari previously requested changes Mar 10, 2021

View reviewed changes

dtassone added 3 commits March 10, 2021 17:08

add col events, cleanup

d1efda9

refactor cells to require id and field

e1cbee8

api docs update

ec2ff82

oliviertassinari reviewed Mar 10, 2021

View reviewed changes

docs/pages/api-docs/x-grid.md Outdated Show resolved Hide resolved

dtassone added 3 commits March 10, 2021 19:19

add tests

837c05a

add event arg in api docs

c3e2120

add more tests

89db2a5

oliviertassinari reviewed Mar 10, 2021

View reviewed changes

packages/grid/x-grid/src/tests/events.XGrid.test.tsx Outdated Show resolved Hide resolved

packages/grid/x-grid/src/tests/events.XGrid.test.tsx Outdated Show resolved Hide resolved

packages/grid/x-grid/src/tests/events.XGrid.test.tsx Show resolved Hide resolved

dtassone requested a review from oliviertassinari March 10, 2021 18:52

oliviertassinari reviewed Mar 10, 2021

View reviewed changes

packages/grid/x-grid/src/tests/events.XGrid.test.tsx Outdated Show resolved Hide resolved

oliviertassinari reviewed Mar 10, 2021

View reviewed changes

packages/grid/x-grid/src/tests/events.XGrid.test.tsx Outdated Show resolved Hide resolved

small refactoring on tests

1e3b652

dtassone requested a review from oliviertassinari March 11, 2021 09:57

oliviertassinari reviewed Mar 11, 2021

View reviewed changes

dtassone and others added 4 commits March 11, 2021 14:21

Add . at the end of desc

f2deb7a

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

Update packages/grid/_modules_/grid/models/api/gridParamsApi.ts

af50f17

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

Update packages/grid/_modules_/grid/models/params/gridCellParams.ts

be8b477

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

small cleanup

a8f25de

dtassone requested a review from oliviertassinari March 11, 2021 13:54

oliviertassinari approved these changes Mar 11, 2021

View reviewed changes

dtassone merged commit 517ed5e into mui:master Mar 11, 2021

oliviertassinari mentioned this pull request Mar 17, 2021

[DataGrid] Checkbox doesn't work in cells #1238

Closed

2 tasks

This was referenced Mar 25, 2021

v4.0.0-alpha.23 #1280

Merged

[CHANGELOG] v4.0.0-alpha-23 #1277

Merged

This was referenced Mar 27, 2021

[DataGrid] Events from cell's portal should be ignored #1305

Closed

[DataGrid] Loose focus when opening a textbox portal inside a cell #1295

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Improve the handling of events (rm capture, add event, add new props) #1158

[DataGrid] Improve the handling of events (rm capture, add event, add new props) #1158

dtassone commented Mar 3, 2021 •

edited

Loading

oliviertassinari left a comment

dtassone commented Mar 5, 2021

oliviertassinari commented Mar 6, 2021 •

edited

Loading

dtassone commented Mar 8, 2021

dtassone commented Mar 9, 2021

oliviertassinari left a comment

oliviertassinari left a comment •

edited

Loading

oliviertassinari left a comment

oliviertassinari left a comment

tusharchutani commented Mar 25, 2021

dtassone commented Mar 26, 2021

[DataGrid] Improve the handling of events (rm capture, add event, add new props) #1158

[DataGrid] Improve the handling of events (rm capture, add event, add new props) #1158

Conversation

dtassone commented Mar 3, 2021 • edited Loading

Breaking Changes

oliviertassinari left a comment

Choose a reason for hiding this comment

dtassone commented Mar 5, 2021

oliviertassinari commented Mar 6, 2021 • edited Loading

dtassone commented Mar 8, 2021

dtassone commented Mar 9, 2021

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari left a comment • edited Loading

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

tusharchutani commented Mar 25, 2021

dtassone commented Mar 26, 2021

dtassone commented Mar 3, 2021 •

edited

Loading

oliviertassinari commented Mar 6, 2021 •

edited

Loading

oliviertassinari left a comment •

edited

Loading