[DataGrid] Add OnCellClick and allow restricting selection to checkboxes

[hksalessio]

Pull Request

📖 Description

This PR adds an OnCellClick functionality to FluentDataGrid and allows restricting the selection of rows to the checkboxes of the select column.

🎫 Issues

This PR should allow for implementing the features requested in #2130 and #1952 (comment).

👩‍💻 Reviewer Notes

Since I renamed FluentDataGridCell.GridColumn to FluentDataGridCell.ColumnIndex (because I think this is more fitting), this PR technically contains a breaking change. If you don't want this change, I will revert it.

✅ Checklist

General

• I have added tests for my changes.
• I have tested my changes.
• I have updated the project documentation to reflect my changes.
• I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

• I have added a new component
• I have added Unit Tests for my new compontent
• I have modified an existing component
• I have validated the Unit Tests for an existing component

[vnbaaij]

Yeah, you definitely need to revert the renaming first. We use that name because it is also used in the web component and we want to keep that (logical) link

[vnbaaij]

Revert GridColumn name change

[dvoituron]

Could you add some unit tests to FluentDataGridColumSelectTests to validate the new RestrictToCheckbox attribute?

[hksalessio]

Could you add some unit tests to FluentDataGridColumSelectTests to validate the new RestrictToCheckbox attribute?

I added some 👍

[dvoituron]

An UI issue occurs when I change the RestrictToCheckbox property

[dvoituron]

The OnRowClick should be raised in all cases: it's correct when RestrictToCheckbox is unchecked.
But is not correct when RestrictToCheckbox is checked.

[hksalessio]

The OnRowClick should be raised in all cases: it's correct when RestrictToCheckbox is unchecked. But is not correct when RestrictToCheckbox is checked.

The problem with this is that I would not see any way to implement #2130 since the row click is always triggered after the cell click. When clicking the checkbox, the clicked row would be selected but if OnRowClick is executed afterwards to deselect all other rows, the previous selection would be cleared (of course, this only applies to multi select mode).

[dvoituron]

If OnRowClick is executed afterwards to deselect all other rows, the previous selection would be cleared (of course, this only applies to multi select mode).

Yes, that was my initial question. I haven't analyzed the problem. But it would be better to have this event in any case. If not, we'll get some "feedback" :-) I can look at it in a few days if you like, see if I have another idea.

[hksalessio]

Maybe we could add a new parameter StopClickPropagation or PreventRowClick to columns?

[dvoituron]

Maybe we could add a new parameter StopClickPropagation or PreventRowClick to columns?

No. That's not ideal. We should find a solution to do that automatically.

[hksalessio]

I can look at it in a few days if you like, see if I have another idea.

Hi, any news on this? 😅

[dvoituron]

Hi, any news on this?

I haven't had time yet :-(

[franklupo]

Normally you select only from the checkbox. the current behavior is not correct. If you want something different you use the envents on the row.

[franklupo]

for #2130 the solution is in the event. Call deselect all and select the current row.

[hksalessio]

Normally you select only from the checkbox. the current behavior is not correct. If you want something different you use the envents on the row.

I definitely agree with you. However, this could be considered a breaking change by the repository owners.

[franklupo]

Normally you select only from the checkbox. the current behavior is not correct. If you want something different you use the envents on the row.

I definitely agree with you. However, this could be considered a breaking change by the repository owners.

the solution might be to create a new selection column that behaves correctly.

[dvoituron]

⚠️ @hksalessio @vnbaaij I created this new PR: #2252
Give me your feedbacks

[vnbaaij]

Other PR LGTM. Merged it and think we can close this one.

[hksalessio]

⚠️ @hksalessio @vnbaaij I created this new PR: #2252 Give me your feedbacks

Looks good to me, thank you!