feat(datagrid): custom select all #1720
Conversation
Thank you, 🤖 Clarity Release Bot |
| <clr-checkbox-wrapper class="clr-dg-custom-select-all"> | ||
| <input clrCheckbox type="checkbox" (click)="toggleSelectAllRows()" /> | ||
| </clr-checkbox-wrapper> |
There was a problem hiding this comment.
There is no label for this checkbox. I don't think applications should be required to provide that or to explicitly provide the checkbox elements. I will propose a different implementation.
There was a problem hiding this comment.
See #1724 for an alternative solution. I think it would be better to provide a clrDgCustomSelectAll event instead of requiring applications is provide a separate checkbox element and anything else needed for that. That will prevent applications from missing the required label for the checkbox.
There was a problem hiding this comment.
Let's move this disscussion in JIRA.
There was a problem hiding this comment.
There is no label for this checkbox. Label is not required for the checkbox.
There was a problem hiding this comment.
@kevinbuhmann there is better solution than one provided in #1724 , but let's first settle on a best approach.
There was a problem hiding this comment.
Label is not required for the checkbox.
What? A label is required for all checkboxes. It needs the label for screen readers.
| @@ -22,7 +22,10 @@ | |||
| *ngIf="selection.selectionType === SELECTION_TYPE.Multi" | |||
| (keydown.space)="toggleAllSelected($event)" | |||
There was a problem hiding this comment.
This is broken with custom select all.
There was a problem hiding this comment.
I fixed this in my proposed implementation.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Datagrid is using internal select all algorithm only.
Issue Number: CDE-2588
What is the new behavior?
Providing a projected
clrCheckboxcan leverage a custom implementation of that logic.Does this PR introduce a breaking change?
Other information