feat(DataTable): add support for pagination #3199
Conversation
🦋 Changeset detectedLatest commit: d401d8a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
joshblack
added
skip changeset
…act into feat/add-pagination-to-table-2
There was a problem hiding this comment.
Nice work on this, @joshblack! Excited for async behavior next ✨
| selectPage, | ||
| selectNextPage, | ||
| selectPreviousPage, | ||
| } = usePagination({ |
There was a problem hiding this comment.
Love how this simplifies the Pagination component ❤️
| return context | ||
| } | ||
|
|
||
| function LiveRegion({children}: React.PropsWithChildren) { |
There was a problem hiding this comment.
Excited about LiveRegion! Can't wait to use this in TreeView
| ) | ||
| } | ||
|
|
||
| function Message({value}: {value: string}) { |
There was a problem hiding this comment.
Is this a declarative API for making announcements?
There was a problem hiding this comment.
@colebemis exactly, still trying to see if it works the way we anticipate 😅 Thankfully everything announces as-intended right now but a little worried about the declarative API translating to side-effects with announcements.
| @@ -0,0 +1,478 @@ | |||
| import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' | |||
There was a problem hiding this comment.
I'm a little worried about keeping this1 in sync with our exported Pagination component.
Do you think it makes sense to do either of these?
- Introduce a new pagination component for use-cases like these
- Do a follow up PR where we update our existing Pagination component to have some of the features you've added here
Footnotes
-
I'm only talking about the page numbers and prev/next buttons. I don't think we have other needs for the "1-10 of 1000" bit yet. ↩
There was a problem hiding this comment.
I definitely think there is something there where we could merge the two components, in particular to share appearance and behavior. It'd be great if we could figure out how to allow customization of the inner pages to allow for the different navigation use-cases (button vs a). I have no idea how to expose this kind of stuff to an end user without coming up with some kind of naming scheme between the two though 🤔
There was a problem hiding this comment.
Should we create an issue to figure this out? Want to discuss it during the next Primer patterns working session?
There was a problem hiding this comment.
Definitely! Made one over at: https://github.com/github/primer/issues/2164 feel free to add to it if I missed anything 👀
Batch https://github.com/github/primer/issues/1890
Add support for pagination within a DataTable. This adds in a
Table.Paginationcomponent along with a storybook story to demonstrate the behavior, presentation, and semantics of pagination alongside a table. This PR does not include support for loading or error states; these will be added in a follow-up PR.Loom walkthrough of the behavior and changes here: https://www.loom.com/share/b2b67e2102074c55a86b2617860639fc
Changelog
New
Table.Paginationand corresponding storybook storyButtonResetLiveRegion,LiveRegionOutlet, andMessageVisuallyHidden