[DataGrid] Ignore event from portal in cells #1324
Conversation
oliviertassinari
added
bug 🐛
|
it should be ok to have the event bubble to the grid container. |
|
I will add a regression test case if we agree with the approach. I didn't dive too much into the codebase, to try to find other events subscribed. What I did instead of click/key around to find any wrong behavior, couldn't spot any. |
|
We should probably link the React issue in the comment, for context. |
|
I will wait for @dtassone to come back before continue the effort in the issue. |
|
You should add a test for renderCell, and portals to make sure it works as expected but the approach looks correct |
|
Ok, will do such |
oliviertassinari
force-pushed
the
data-grid-cell-portals
branch
from
30b3272
to
d204dc2
Compare
oliviertassinari
changed the title
|
@dtassone done |
| @@ -57,4 +63,30 @@ describe('<DataGrid /> - Rows', () => { | |||
| expect(getColumnValues()).to.deep.equal(['Mike-11', 'Jack-11', 'Mike-20']); | |||
| }); | |||
| }); | |||
|
|
|||
| it('should ignore events coming from a portal in the cell', () => { | |||
There was a problem hiding this comment.
Not sure we have a test that shows events from a rendercell without portal
There was a problem hiding this comment.
What do you mean?
There was a problem hiding this comment.
well you check that the event is blocked with portal. I don't think we have a test that check if the event go through without portal.
There was a problem hiding this comment.
Ok, I can add test coverage for this case.
oliviertassinari
force-pushed
the
data-grid-cell-portals
branch
from
d204dc2
to
e547bd2
Compare
|
The React's issue: facebook/react#11387. |
Closes #1295.
Closes #1305.
This seems to cover all the issues. Assuming we will never add another source, maybe it's enough? Opened as a draft to run the CI and collect feedback.