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

feat: use Astro's built-in pagination #376

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

DerTimonius
Copy link
Contributor

Description

This PR implements the built-in pagination of Astro instead of the current self-made. Don't get me wrong, the self-made pagination is great, but I found that sometimes it might be hard to find the correct file in the pages directory that causes an issue.

The upside is that getting the next and previous urls is much easier and does not need manual string concatenation.

I also removed the files with the helper functions that facilitated the previous pagination.

Closes #370

Copy link
Owner

@satnaing satnaing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some issues.
I got the following error when I build the project.

`Astro.request.headers` is unavailable in "static" output mode, and in prerendered pages within "hybrid" and "server" output modes. If you need access to request headers, make sure that `output` is configured as either `"server"` or `output: "hybrid"` in your config file, and that the page accessing the headers is rendered on-demand.

There are some questions while solving the above issue

  • Is it possible to make the warning disappear without making "server" or "hybrid" output?
  • Is there any other way to deal with this without making any redirects?

Another thing is that the type of page is being Page<any>.
We can make the type a bit more strict.

I can co-author this if you don't want to do TS related stuff. Or you can do it yourself if you want. I don't mind either way 🤷

@satnaing
Copy link
Owner

Turns out both can be solved easily.

@satnaing
Copy link
Owner

Hell @DerTimonius,
Do you want to update yourself?
If you need my help, plz do let me know.

@DerTimonius
Copy link
Contributor Author

@satnaing sorry, a lot of work today. I can update this myself, probably tomorrow afternoon, or on the weekend. I'll try to tackle the typescript shenanigans myself 😄

@DerTimonius
Copy link
Contributor Author

DerTimonius commented Sep 14, 2024

Is this the approach you had im mind, @satnaing?

@satnaing
Copy link
Owner

satnaing commented Sep 16, 2024

LGTM
Thanks for your contribution.

@satnaing satnaing merged commit 08b1676 into satnaing:main Sep 16, 2024
1 check 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.

feature request: use Astros built-in pagination
2 participants