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

Nesting PaginationLink/PaginationPrevious/PaginationNext inside PaginationItem produces nested li elements #2154

Closed
dngpng opened this issue Dec 23, 2023 · 6 comments
Labels

Comments

@dngpng
Copy link

dngpng commented Dec 23, 2023

The Issue

The example given in the docs for Pagination component wraps PaginationLink / PaginationPrevious / PaginationNext components with a PaginationItem. However the code for PaginationLink actually already wraps the content inside a PaginationItem, the example will therefore result in a nested PaginationItems (meaning nested li-elements) -- this will lead to React reporting an error during rendering.

image

Possible Solutions

  1. Do not internalize PaginationItem inside a PaginationLink, this way we will have a uniform behavior when constructing a Pagination tree.

  2. Fix the example in the doc so that PaginationLink / PaginationPrevious / PaginationNext are used directly without being wrapped (again) in a PaginationItem

@plsankar
Copy link

Yes, can confirm it produces nested li tags.

Screenshot 2023-12-24 at 1 15 13 PM

@stathis1998
Copy link

I have the same issue. I will try and make a PR.

@HR555
Copy link

HR555 commented Jan 27, 2024

Any update on this ?

Currently as a workaround, I have replaced the PaginationLink with custom PaginationButton components. That solves the problem for now.

css
"inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 hover:bg-accent hover:text-accent-foreground h-9 w-9"

Screenshot 2024-01-28 at 00 46 13 Screenshot 2024-01-28 at 00 50 33

@stathis1998
Copy link

Any update on this ?

Currently as a workaround, I have replaced the PaginationLink with custom PaginationButton components. That solves the problem for now.

css
"inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 hover:bg-accent hover:text-accent-foreground h-9 w-9"

Screenshot 2024-01-28 at 00 46 13

This has been resolved if I am not mistaken. It was merged on another's guy PR.

@HR555
Copy link

HR555 commented Jan 28, 2024

Yes, #2416

@shadcn shadcn added the Stale label Feb 12, 2024
@shadcn
Copy link
Collaborator

shadcn commented Feb 20, 2024

This issue has been automatically closed because it received no activity for a while. If you think it was closed by accident, please leave a comment. Thank you.

@shadcn shadcn closed this as completed Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants