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

Feature/pagination #75

Merged
merged 8 commits into from
Nov 5, 2024
Merged

Feature/pagination #75

merged 8 commits into from
Nov 5, 2024

Conversation

capetillo
Copy link
Collaborator

@capetillo capetillo commented Nov 5, 2024

FEATURE: Pagination for faster thumbnail loading

Background:

When the Dashboard first loaded, it would take a really long time to fetch for the thumbnails (and most of the time it would time out).

Implementation:

  • loadThumbnailsForPage: This loads and displays thumbnails for sessions on the current page. It calculates the start and end indices based on currentPage and pageSize, clears the existing thumbnailsMap, and asynchronously fetches thumbnails for each session. This function will make the request to getThumbnails only 5 times per page (i.e. pageSize)
  • sessionsWithThumbnails: Updated to filter and return sessions for the current page that have associated thumbnail urls. Aka only sessions with available thumbnails are displayed
  • Added a watcher for filteredSessions because when the component first mounts it's an empty array and that would not trigger the cascade of loadThumbnailsForPage --> getThumbnails

Two other small fixes

  • The url to fetch pending requests was missing an & in the obsPortalData store
  • When the user would log in, the log in button would stay until the user refreshed the site. That has been fixed by making the username a computed property in App

VISUALS

User logs in, username is on the right (rather than log in button) and thumbnails load quickly. This is also after clearing my cache so it's reliable!

Screen.Recording.2024-11-05.at.1.31.46.PM.mov

Copy link

@jnation3406 jnation3406 left a comment

Choose a reason for hiding this comment

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

It kind of looks like anytime there is a change in session (new session added or whatever) it will try to reload the thumbnails for all sessions, not just the new ones added? I guess that's fine for now but we should make sure this doesn't make too many calls and take to long for people with large numbers of requests, or large numbers of thumbnails per request.

const getThumbnails = async (observationId) => {
await fetchApiCall({
url: configurationStore.thumbnailArchiveUrl + `thumbnails/?observation_id=${observationId}&size=large`,
url: `${configurationStore.thumbnailArchiveUrl}thumbnails/?observation_id=${observationId}&size=large`,

Choose a reason for hiding this comment

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

Maybe I'm missing something, but if these thumbnails are for the page where it lists your sessions and all their little thumbnails under the dates then I think it should use the small thumbnails, not the large ones. The large ones are like 1/2 your screen size so I would save those for like opening a single image into a larger view...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okey dokes! I should really start testing on the ipad

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and to respond to your top comment, I can definitely add the limit query param to the call. But I'm not sure what the solution could be. Maybe we talk about this in zoom

@capetillo capetillo merged commit 35e5987 into main Nov 5, 2024
1 check passed
@capetillo capetillo deleted the feature/pagination branch November 5, 2024 23:53
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