IgxGrid: contextMenu should fire for the entire row

[Timmeeeey]

Is your feature request related to a problem? Please describe.

When using a context menu in IgxGrid then the context menu only works when a cell is clicked. If you click on a row to the right of the last cell, then the default browser context menu opens.

This is a bad user experience because you have to be careful not to click too far to the right.

Describe the solution you'd like

The contextMenu event should fire for the entire row and not just for cells.

Additional context

[kdinev]

@Timmeeeey The event is firing with arguments for the current cell that the menu is fired on. If we trigger the context menu event without a cell, then the event arguments will be null.

[Timmeeeey]

@kdinev Would it be possible to extend the event arguments with the clicked row? It can be really confusing for a user if the context menu doesn't work for the entire row.

[kdinev]

@Timmeeeey Currently, the row is reachable through the cell in the event arguments, but there's no individual argument for the row only. We can take a look at whether exposing this would be an issue. Our assumption has always been that the visible cells would span the length of the row, which is not the case in the scenario you have.

[Timmeeeey]

@kdinev Thank you. I hope this will be supported in a future release.

[github-actions[bot]]

There has been no recent activity and this issue has been marked inactive.

[Timmeeeey]

@kdinev @ChronosSF Hi, since there is now a rowClick event (see #13755), will the contextmenu event also be supported for the entire row?

[ChronosSF]

@Timmeeeey ,

They are not related necessarily but having contextMenu fire on parts of the rows that don't have cells could have some uses. For example, it could bring out a menu with row actions such as pin and delete. On the other hand we do consider grids with empty space a bit of an edge case, as the vast majority of real-world apps try to not have such empty space at all.

In either case we have to introduce this change without breaking existing apps. We might need to have the type of the event arguments be union between row and cell ones instead of one combining the two unless we can come up with proper migrations. I'll look into this a bit more this sprint.

[github-actions[bot]]

There has been no recent activity and this issue has been marked inactive.

[kdinev]

@ChronosSF Any change someone can look at this?

[ChronosSF]

@kdinev , I created a PR. It was more or less ready for some time but unfortunately as the event arg is cell based, it either requires a type change (breaking) or an union type that would not work in Blazor. I was leaning towards not doing it at all because of these. Didn't get strong opinions for or against either option from the team. Check the PR out and see if you are okay with an union type and basically accepting that it won't work in Blazor. I don't like the idea of changing the type to a single one that either has a cell or row property with a value.

[kdinev]

@kdinev , I created a PR. It was more or less ready for some time but unfortunately as the event arg is cell based, it either requires a type change (breaking) or an union type that would not work in Blazor. I was leaning towards not doing it at all because of these. Didn't get strong opinions for or against either option from the team. Check the PR out and see if you are okay with an union type and basically accepting that it won't work in Blazor. I don't like the idea of changing the type to a single one that either has a cell or row property with a value.

Let's do a type change with a migration.