[DataGrid] Pressing Escape should close GridColumnHeaderMenu without errors

[DanailH]

Found a regression that is a quick win.

Currently pressing the Escape key when the GridColumnHeaderMenu is opened results in an error in the console and the menu doesn't close. This PR fixes the broken behavior.

[flaviendelangle]

In GridColumnHeaderMenu, could we type the params of hideMenu ?
GridMenu tells us that the event can be null but the hideMenu is doing like if it was never null.

[oliviertassinari]

It would be interesting to dive into why we need the event.

[DanailH]

It would be interesting to dive into why we need the event.

event.stopPropagation();

That is why it is breaking. The comment explanation is 'Prevent triggering the sorting'. This is the commit that introduced the change DanailH@382e84b#diff-ac3c8d72bf2d6735fd89cf6d93298a21dce356400957fac9b93263b7c61eb381R30

[oliviertassinari]

Ok, Interesting, the event.stopPropagation purpose seems to be backward. From what I understand.

• Calling event.stopPropagation(); when the Escape key is pressed could help avoid double handling of the event. We use it in the Modal. Maybe we should normalize it. The alternative is to not call it, but set event.defaultMuiPrevented = true.
• Calling event.stopPropagation(); so it doesn't sort doesn't work, it's a dead logic, I can still sort. Worse, it might prevent developers to listen to the event.

[m4theushw]

Calling event.stopPropagation(); so it doesn't sort doesn't work, it's a dead logic, I can still sort. Worse, it might prevent developers to listen to the event.

@oliviertassinari It's working. Its purpose is for when a menu item is clicked it doesn't sort the respective column too. What we could do to not prevent every event is to only call event.preventDefault(); on non-keyboard events.

[oliviertassinari]

@m4theushw Ohh, right, I see. My bad.