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

Add pagination support to bookmark context #350

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shayesdevel
Copy link

@shayesdevel shayesdevel commented Feb 26, 2025

  • Implemented infinite scrolling with IntersectionObserver for better performance
  • Added pagination API endpoint support in the frontend
  • Updated BookmarkContext to support loading bookmarks in chunks as user scrolls
  • Created tests for the pagination functionality
  • Fixed compatibility with existing code and handling of tags/filtering

🤖 Generated with Claude Code
Co-Authored-By: shayesdevel@gmail.com

Issue number: resolves #


Checklist

  • Code Formatter (run prettier/spotlessApply)
  • Code has unit tests? (If no explain in other_information)
  • Builds on localhost
  • Builds/Runs in docker compose

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

  • Yes
  • No

Other information

- Implemented infinite scrolling with IntersectionObserver for better performance
- Added pagination API endpoint support in the frontend
- Updated BookmarkContext to support loading bookmarks in chunks as user scrolls
- Created tests for the pagination functionality
- Fixed compatibility with existing code and handling of tags/filtering

🤖 Generated with Claude Code
Co-Authored-By: shayesdevel@gmail.com
@R-Sandor
Copy link
Owner

R-Sandor commented Mar 5, 2025

Please fix the following behaviors:

  • Successful bookmark toast does not propagate, in fact no bookmark appears to be added to the to view if the add was a success.
  • Once a bookmark is added, scrolling to get the next page doesn't produce the new bookmarks.
    • User must refresh and scroll.
  • Tag counts don't increment when a bookmark is added.
  • The scroll is a little chuggy, ideally we want to get the next page before the user feels the lag of that request.
  • Search is broken

Copy link
Owner

@R-Sandor R-Sandor left a comment

Choose a reason for hiding this comment

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

Review behavioral feedback mentioned before the PR. Overall code looks very well planned, the observer needs to load the next page before the entire scroll to the bottom.

@@ -69,6 +69,12 @@ const api = {
getAllBookmarks() {
return this.execute("GET", "bookmarks", null, {});
},
// Get paginated bookmarks.
getPaginatedBookmarks(page: number, size: number = 10) {
Copy link
Owner

Choose a reason for hiding this comment

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

This looks good.

if (typeof IntersectionObserver !== 'undefined') {
const observer = new IntersectionObserver(handleObserver, {
root: null,
rootMargin: '20px',
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe make this a little bit bigger because its not getting the next page until I am well scrolled down and it lags.

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.

2 participants