Skip to content

Add datatable checkbox component/story and improve TabBar story docs #149

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 5, 2022
Merged

Add datatable checkbox component/story and improve TabBar story docs #149

merged 4 commits into from
Jul 5, 2022

Conversation

@hobbitronics
Copy link
Contributor

@hobbitronics hobbitronics commented Jun 29, 2022 •
edited
Loading

  • added Datatable.Header.Checkbox and Datatable.Checkbox subcomponents
  • Datatable now emits events for checkboxes (as opposed to how we were using checkbox in cover)
  • added to and improved story/documentation to show subcomponents in Datatable and TabBAr
  • docs now renders all the subcomponents but shows {...args} instead of props for Datatable and TabBar
  • added numberOfCheckboxes to Datatable to register new Checkboxes which fixes an exception

@hobbitronics hobbitronics requested a review from a team June 29, 2022 05:46
@hobbitronics hobbitronics changed the title Add datatable checkbox and improve TabBar story Add datatable checkbox components/story and improve TabBar story docs Jul 1, 2022
@hobbitronics hobbitronics changed the title Add datatable checkbox components/story and improve TabBar story docs Add datatable checkbox component/story and improve TabBar story docs Jul 1, 2022
forevermatt
forevermatt reviewed Jul 1, 2022
View reviewed changes
components/mdc/Datatable/Datatable.svelte Outdated
dispatch('rowSelectionChanged', event.detail)
})
// This does not work because of an MDC bug. See https://github.com/material-components/material-components-web/issues/6385
Copy link
Contributor

@forevermatt forevermatt Jul 1, 2022

Choose a reason for hiding this comment

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

Looks like this comment may need to be updated. That bug seems to have been fixed, as I'm guessing you found since you uncommented the destroy line.

Copy link
Contributor Author

@hobbitronics hobbitronics Jul 5, 2022

Choose a reason for hiding this comment

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

I forgot to delete that. The issue was fixed.

forevermatt
forevermatt reviewed Jul 1, 2022
View reviewed changes
components/mdc/Datatable/DatatableCheckbox.svelte Outdated

<td class="mdc-data-table__cell mdc-data-table__cell--checkbox" on:click>
<div class="mdc-checkbox mdc-data-table__row-checkbox">
<input type="checkbox" class="mdc-checkbox__native-control" aria-labelledby={inputID} {disabled} bind:checked />
Copy link
Contributor

@forevermatt forevermatt Jul 1, 2022

Choose a reason for hiding this comment

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

This aria-labelledby seems like it's referring to some other element with that ID, but that inputID isn't used anywhere else. I might be missing something, but it seems like something is missing.

Copy link
Contributor Author

@hobbitronics hobbitronics Jul 5, 2022

Choose a reason for hiding this comment

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

You are correct. It should be the same as Datatable.Data.Row rowId so I will add a slot prop to expose rowId and we can use the let directive to bring it in here as a prop.

forevermatt
forevermatt approved these changes Jul 1, 2022
View reviewed changes
Copy link
Contributor

@forevermatt forevermatt left a comment

Choose a reason for hiding this comment

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

Two comments, neither blocking.

@hobbitronics hobbitronics force-pushed the feature/add-Datatable-checkbox branch from 143ef10 to 01aa5d0 Compare July 5, 2022 04:34
@hobbitronics hobbitronics merged commit 34a24df into develop Jul 5, 2022
@hobbitronics hobbitronics deleted the feature/add-Datatable-checkbox branch July 5, 2022 04:39
@hobbitronics
Copy link
Contributor Author

hobbitronics commented Jul 5, 2022

🎉 This PR is included in version 6.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@forevermatt forevermatt forevermatt approved these changes

Assignees

No one assigned

Labels

released

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hobbitronics @forevermatt