Skip to content

fix(Pagination): Allow usage of Pagination component with Next.js #273

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

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

DavidFrancois
Copy link
Contributor

@DavidFrancois DavidFrancois commented Jul 10, 2024

ATM when on the first or last page of the pagination component, a Link component is generated with no href attribute for the first / last page and previous / next page elements of the pagination.

The issue seem to have been running for almost a year now #170

Issue : Since NextJS can't handle Link without an href attribute, the whole page crashes if we're on the first or last page of the pagination component.

Steps to reproduce : Clone react-dsfr, use a Pagination component in test/integration/next-appdir/ui/ClientComponent.tsx.

Set defaultPage attribue to either 1 or the same as count count attribute, and count > 1.
Run and try to access the page.

example :

<Pagination
       getPageLinkProps={(page) => ({ href: `?page=${page}` })}
       count={10}
       defaultPage={1}>
</Pagination>

Solution :
I tried to solve this in the most simple and straight forward way :
If we're on either the first or last page of the Pagination component, then we use a <a> HTML element instead of <Link> for those specific action links.

Since the link is not clickable anyway in such case, this should not cause any issue or regression.

@martinratinaud
Copy link

Need this ASAP :-) Thanks a lot

@garronej
Copy link
Collaborator

Thank you for taking the time to submit a PR!

@garronej garronej merged commit fb4a2f0 into codegouvfr:main Jul 10, 2024
6 checks passed
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.

3 participants