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: Pagination #144

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

feat: Pagination #144

wants to merge 3 commits into from

Conversation

ManorSailor
Copy link
Contributor

@ManorSailor ManorSailor commented Sep 16, 2024

Closes #15

Bugs:

  • Search is broken.

Suggestions Needed:

  • Should we add a Homepage that displays a few cards, along with a link to page 1 of the terms?
  • Regarding the Search feature, I’m open to suggestions, as I currently don’t have a clear idea on how to fix it.

Tradeoffs:

  • I decided to keep the pagination buttons (previous & next) visible in their respective edge cases (previous button visible on the first page, and next button visible on the last page) instead of conditionally rendering them. This is because conditionally rendering was causing styling issues when an element was absent from the page.
  • Initially, I considered disabling the a tags; however, since they don't support the disabled attribute, I added a new style to disable them via CSS. This is, perhaps, not the most accessible approach.
  • I also decided to keep the root route, i.e., the homepage, intact even though it currently lacks any content. This will make it easier to add a homepage in the future.

@ManorSailor ManorSailor changed the title Pagination Support feat: Pagination Sep 16, 2024
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.

Pagination (suggestion)
1 participant