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

feat: add core advanced row selection demo #12

Merged
merged 1 commit into from
Jul 21, 2022
Merged

Conversation

steve-haar
Copy link
Contributor

No description provided.

@steve-haar steve-haar self-assigned this Jul 19, 2022
@steve-haar steve-haar marked this pull request as ready for review July 20, 2022 16:35
@steve-haar steve-haar marked this pull request as draft July 20, 2022 16:39
@steve-haar steve-haar marked this pull request as ready for review July 20, 2022 22:13
>
<cds-grid-column type="action">
<cds-checkbox (click)="stopPropagation($event)">
<input (click)="selectAll()" type="checkbox" [checked]="data.entirePageSelected" aria-label="Select All" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this have an indeterminate in there?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we talked about this yesterday, but is this even something we should have here? It's certainly not scalable to fetch all of the remote data to track all the items as selected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check-all behavior can't be turned off in the angular grid and the angular grid doesn't have an indeterminate state. I thought they should behave identically, so this will make the core grid behave just like the angular grid.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is benefit in showing slightly different behavior if there is something you can do in Core that you can't do in NG, but the checkbox is probably not important enough for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge for now, but we can pretty easily add indeterminate behavior if we want.

[attr.selected]="data.selectedMap![vm.id]"
>
<cds-grid-cell>
<cds-checkbox (click)="stopPropagation($event)">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a weird hack, hmm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd love any insights you can give. If I use a regular input checkbox (not wrapped in a cds-checkbox), then everything works fine. When I use the cds-checkbox, I have to add this otherwise it won't work properly. Do you have any idea what's going on?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't, I can take a look after it's merged in. Does it not work at all? Or is it flakey? I noticed some flakiness in the grid in the core storybook, that I hadn't investigated yet, but for the most part it worked fine in vanilla web components.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it has something to do with the cds-checkbox and not the grid (just my guess). The behavior is consistent and the select-all doesn't work at all if I don't have the stopPropagation.

Signed-off-by: Ashley Ryan <asryan@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants