Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DetailRowTrigger on DataGrid is called twice on each row click #2500

Closed
GilShalit opened this issue Jun 19, 2021 · 8 comments · Fixed by #3008
Closed

DetailRowTrigger on DataGrid is called twice on each row click #2500

GilShalit opened this issue Jun 19, 2021 · 8 comments · Fixed by #3008
Assignees
Labels
Type: Bug 🐞 Something isn't working
Milestone

Comments

@GilShalit
Copy link

GilShalit commented Jun 19, 2021

Blazorise version 0.9.3.9

Using the following to close a details row if the row is clicked a second time

<DataGrid TItem="CourtCase"
		  @bind-SelectedRow="@selectedCourtCase"
		  DetailRowTrigger="@((item)=> item.Id == selectedCourtCase?.Id && !isRowAlreadySelected())">
...
</DataGrid>

with

public CourtCase selectedCourtCase;        
CourtCase prevSelectdRow;

bool isRowAlreadySelected()
        {
            bool isRowClicked = prevSelectdRow!=null && prevSelectdRow.Id == selectedCourtCase.Id;
            prevSelectdRow = selectedCourtCase;
            return isRowClicked;
        }

Does not work - details are never opened. Using debug I found that isRowAlreadySelected() is called twice on each row click, so obviously on the second call the prevSelectedRow variable is already set to the row which is newly selected and so the result is effectively always true.

I found a way around this with an additional flag, but I think this isn't how it should work.

@stsrki
Copy link
Collaborator

stsrki commented Jun 19, 2021

I would say this is good behavior. DetailRowTrigger can be potentially called for every row in a table. That is why it has item as an argument. It's not just for the currently selected row.

@GilShalit
Copy link
Author

Right. But for some reason it seems to be called twice with the same item and selectedCourtCase because the first part of the trigger condition is met twice so that isRowAlreadySelected() is visited twice for each row click.

@StephenOTT
Copy link

Having similar issues: This seems to fit into the Virtualization work for the Datagrid?

@David-Moreira
Copy link
Contributor

The detail row should close with Ctrl+click, have you guys tried that? Or does that not satisfy your use case?

@GilShalit
Copy link
Author

Yes it works with Ctrl+Click, but this does not seem as intuitive as simply have the second click close an opened row. How do I know it is not intuitive? EVERY new user asks me why isn't the second click closing the opened row...

@stsrki
Copy link
Collaborator

stsrki commented Jun 27, 2021

@David-Moreira do you think we should change the toggle with a click instead of ctrl+click?

@David-Moreira
Copy link
Contributor

Well, @GilShalit does have a point. I would say most users, would just expect to just click back again and it would collapse the DetailRowTrigger .
However the reason why Ctrl+Click works, is because it will unselect the row, therefore the DetailRowTrigger Func will not evaluate to true anymore.

I think it's feasible.
At the moment DetailRowTrigger is evaluated on a per blazor state update basis. Maybe we could start evaluating it only on the actual clicks, that should also solve it being possibly triggered more than once due to blazor state updates.

  • Note: Need to check how it behaves with MultiSelectAll and if we actually do need it to evaluate on blazor state updates for some reason.

@stsrki let me know your thoughts.

@stsrki stsrki added this to the Last Minute milestone Jun 27, 2021
@stsrki
Copy link
Collaborator

stsrki commented Jun 27, 2021

@David-Moreira I didn't try it but multi-select might be problematic indeed.

I will assign it to you. Also, I have placed in the new Last Minute milestone. It means, in case we got enough time before the 0.9.4 release you can try to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐞 Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants