-
-
Couldn't load subscription status.
- Fork 366
feature: First Page and Last Page buttons for svelte #3987
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 742f113 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
I screwed up the docs example, I put the wrong icon in |
|
@Uncle798 I'll let @Hugos68 do a sweep as well. But overall looks like a good start, just a few things of note:
If you need any specific help, ping Hugo or myself on Discord. I'm in and out so Hugo may respond faster though. Recommended changeset description: |
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.
Some small things. We should also aim to add two tests and add the React version.
| const attributes = $derived( | ||
| mergeProps( | ||
| { | ||
| onclick: pagination().goToFirstPage() | ||
| }, | ||
| { | ||
| class: classesPagination.nextTrigger, | ||
| }, | ||
| rest, | ||
| ), | ||
| ); |
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.
You can actually put this next to class:
| const attributes = $derived( | |
| mergeProps( | |
| { | |
| onclick: pagination().goToFirstPage() | |
| }, | |
| { | |
| class: classesPagination.nextTrigger, | |
| }, | |
| rest, | |
| ), | |
| ); | |
| const attributes = $derived( | |
| mergeProps( | |
| { | |
| class: classesPagination.nextTrigger, | |
| onclick: pagination().goToFirstPage() | |
| }, | |
| rest, | |
| ), | |
| ); |
| Provider: RootProvider, | ||
| Context: RootContext, | ||
| PrevTrigger: PrevTrigger, | ||
| FirstTrigger: FirstTrigger, |
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.
Please put it in front of Prev as it's First, Prev, Next, Last
|
@Uncle798 Looks like you need to run the formatter, and the deployment on Vercel is failing because of: Make sure you are defining and exporting the 2 newly added interfaces from the components. |
packages/skeleton-svelte/src/components/pagination/anatomy/last-trigger.svelte
Outdated
Show resolved
Hide resolved
| mergeProps( | ||
| { | ||
| class: classesPagination.nextTrigger, | ||
| onclick: pagination().goToFirstPage() |
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.
Make sure to put this in it's own closure, now it will be called once and the return value is passed to onclick, you would want. This shouldn't work.
Linked Issue
n/a
Description
Adding first page and last page button for Pagination for Svelte
Checklist
Please read and apply all contribution requirements.
docs/,feature/,chore/,bugfix/mainbranchpnpm checkin the root of the monorepopnpm formatin the root of the monorepopnpm lintin the root of the monorepopnpm testin the root of the monorepo/packageprojects, please supply a ChangesetChangesets
View our documentation to learn more about Changesets. To create a Changeset:
pnpm changesetand follow the prompts