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

Next/Previous photo sometimes fails now we have pagination #256

Closed
damianmoore opened this issue May 8, 2021 · 7 comments
Closed

Next/Previous photo sometimes fails now we have pagination #256

damianmoore opened this issue May 8, 2021 · 7 comments

Comments

@damianmoore
Copy link
Collaborator

There is a photos Redux store which contains the photo IDs from the timeline view. This is the basis for determining the next and previous photo to go to from the detail screen. Now that not all photos are contained in the store because of pagination, it's possible the current photo ID is not in the store. This is most apparent when getting to photo detail from the map view. If we came from the map view then next/previous should still get the photo taken before or after.

The selector ui/src/stores/photos/selector.js should be updated so if the ID cannot be found or there is are no prev or next results it makes a GraphQL query. This query could be the same one that makes the timeline currently. The currently active search filters should still apply to this fetch. As there is an after parameter added for pagination, a before one should be added and these can both have limit set to 1.

@damianmoore damianmoore added this to the 1.0 milestone May 8, 2021
@damianmoore damianmoore added this to To do in Development tasks May 8, 2021
@damianmoore damianmoore changed the title Next/Previous photo fails now we have pagination Next/Previous photo sometimes fails now we have pagination May 10, 2021
@GyanP
Copy link
Collaborator

GyanP commented May 10, 2021

@damianmoore Could you please help me to regenerate the issue by explaining about this on little-bit detail?

@damianmoore
Copy link
Collaborator Author

@GyanP The pagination loads 100 photos at a time so you'll need to load in more than that number of photos.

  1. Select a photo that is near the end of the first 100 (careful not to trigger the pagination to load the next page)
  2. Click the "next" arrow on the right side of the screen or press right arrow key on keyboard
  3. Note that once you get to the last photo of the page it doesn't go to the next photo as it would be in the next page.

@GyanP
Copy link
Collaborator

GyanP commented May 11, 2021

Thanks @damianmoore

  • I will load 1000 photo per page.
  • As i generate the issue on my system so you mean the pagination not working on detail page am i right?
  • And it should load next page photos(or hit query to load next photos) when we are on last photo of any page.

@damianmoore
Copy link
Collaborator Author

damianmoore commented May 11, 2021

Hi @GyanP,

  • Yes this is on the PhotoDetail screen - the arrows on the left and right.
  • Don't try to load the whole next page of photos as the user might have come from the map view page - Just update the ui/src/stores/photos/selector.js as mentioned above to fall back to getting the 1 result previous and next if there is nothing available in the store.

@GyanP
Copy link
Collaborator

GyanP commented May 11, 2021

@damianmoore Currently we are updating the store by data of page for example:-
limit per page = 50
1.So first 50 photo stored in our store when graphql query hits first time(when first time page load).
2.Then next 50 will be fetched by graphql query and added to store if user scroll down to bottom.
3.repeat 2nd step.

@GyanP
Copy link
Collaborator

GyanP commented May 11, 2021

As we have provided the after parameter which stores the cursorValue of last(end) photo of page.
And on the basis of that cursorValue our graphql query fetch next 50 photos.

@GyanP GyanP moved this from To do to In progress in Development tasks May 20, 2021
@damianmoore damianmoore moved this from In progress to Review in progress in Development tasks Jul 12, 2021
@damianmoore
Copy link
Collaborator Author

Merged as of 137987d. Will be included in v0.22.0.

Development tasks automation moved this from Review in progress to Done Aug 2021 Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development tasks
Done Aug 2021
Development

No branches or pull requests

2 participants