-
Notifications
You must be signed in to change notification settings - Fork 8
Added Select All toggle #262
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution 👍 . I did some checks and would like you to change some minor stuff:
-
The "Select All" checkbox doesn't react to a track beeing unselected. As a user I would expect a state like "partly selected":
-
The code can be shortened:
<Check TValue="bool" CheckedChanged="SelectAllTracks" Checked="selectedTracks.Any()" />
void SelectAllTracks(bool select)
{
if(select)
{
selectedTracks.AddRange(Cuesheet.Tracks);
}
else
{
selectedTracks.Clear();
}
}
- The method "SelectAllTracks" should be starting with "On", for example "OnSelectAllTracksChanged".
If you could edit these things, I will include the pull request. I would be happy to see more contributions from you, so if you want, have a look around the issues ;).
Sure no problem, I'll fix it and create a new pull request. |
Hey, I changed the Select All functionality as you wanted and fixed some bugs, you forgot to remove a track from the selectedTracks list when removing a track from Cuesheet.Tracks in some places. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution and the fixes in code! I really appriciate this :).
What I am missing is the feature of the new select all, that not all tracks are selected. I wasn't clear I see, so I have a screenshot here from blazorise examples:
The example can be found here: https://bootstrap5demo.blazorise.com/tests/datagrid
This should be included and then the feature is done :).
Okay, no problem. I hope everything is now as you wanted. |
Thanks for your contribution, now everything works fine. Also thanks for fixing bugs, I'll look forward for more contributions from you :). |
Hello, I have successfully added the select all toggle to the tracks table.
Closes #252