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

Fix fixed items pagination #464

Merged
merged 5 commits into from
Aug 21, 2024
Merged

Conversation

madaxen86
Copy link
Contributor

Added a fix for the case when paginations has fixedItems and the count is "small" which lead to

negative previousSibling pages
higher nextSibling pages than count
Added to render the pages directly if there's no need to render an ellipsis, because it would only replace one page.
All test still pass.

Side note: I converted the signals and createEffect to a single createMemo to avoid endless update loops.
Ryan Carniato once said in one of his talks that you should never set a signal from within createEffect and that it usually can be converted to a computed value (createMemo, createComputed,...) instead.

Feel free to reach out and discuss.
Spent a whole day, to figure it out. In the end it was so simple 😅

Closes: Issue 461

Copy link

netlify bot commented Aug 16, 2024

Deploy Preview for kobalte ready!

Name Link
🔨 Latest commit 89c003e
🔍 Latest deploy log https://app.netlify.com/sites/kobalte/deploys/66c63ba5f880ae00083670eb
😎 Deploy Preview https://deploy-preview-464--kobalte.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jer3m01 jer3m01 self-assigned this Aug 18, 2024
…ngs count for low total count

as no ellipses are necessary if the total count is lower than 2x siblingCount + boundaries and
potential when items are fixed, we can just render only the count directly.

fix kobaltedev#461
…more than one item - otherwise add item to corresponding sibling

little refactor:
simplified condition for renderItemsDirectly
@madaxen86
Copy link
Contributor Author

Made biome to format and lint and made minor change to pagination-items.tsx
also added changeset to PR

@madaxen86
Copy link
Contributor Author

Added another commit which replaces the ellipsis if it would replace only one pagination item.

madaxen86 and others added 2 commits August 20, 2024 17:09
happened when last item of a page was removed - resulting in fewer pages and 'Invalid array length'
@jer3m01 jer3m01 merged commit c0028e5 into kobaltedev:main Aug 21, 2024
5 of 7 checks passed
@jer3m01
Copy link
Member

jer3m01 commented Aug 21, 2024

Thanks for the fix!

@madaxen86 madaxen86 deleted the pagination-fix branch August 22, 2024 11:03
@madaxen86
Copy link
Contributor Author

Thanks for the fix!

You're welcome. But the changes don't show up on the npm package. Should I have changed the version in the package.json?

@jer3m01
Copy link
Member

jer3m01 commented Aug 22, 2024

But the changes don't show up on the npm package. Should I have changed the version in the package.json?

Haven't released an update yet, will take care of it soon.

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.

Bug in Pagination component if count < 6
2 participants