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

Navigation buttons on landing page #1172

Merged
merged 8 commits into from
Aug 1, 2022
Merged

Conversation

DeepikaGhodki
Copy link
Contributor

@DeepikaGhodki DeepikaGhodki commented Jul 8, 2022

Issue 775

Proposed Solution:
We add prev and next buttons on the top right within the search toolbar as indicated in the screenshot below:
Screenshot from 2022-07-08 17-20-33

  • The search parameters (query, sorting) would be inherited from the list page as query params
  • Implemented using <v-pagination> with a page size of 1
  • Any navigation event would trigger a new fetch request with a page size of 1 and page number = position of the current item. The position is calculated wrt the entire result set i.e. page number * no of items per page + position on current page

Limitations

  • Query parameters may not be persisted on directly opening a DLP

TODO

  • Fix the sorting order (not being inherited from the list page)
  • Remove unnecessary props/ duplicate code
  • Check if this interferes with the existing watch methods for version and identifier as they also lead to page navigation
Not Found
@DeepikaGhodki
Copy link
Contributor Author

For starters, I've placed the pagination buttons to the right of search toolbar. @waxlamp, what do you think?

@DeepikaGhodki DeepikaGhodki changed the title Pagination on landing page Navigation buttons on landing page Jul 14, 2022
@DeepikaGhodki DeepikaGhodki marked this pull request as ready for review July 14, 2022 14:46
Copy link
Member

@jjnesbitt jjnesbitt left a comment

Choose a reason for hiding this comment

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

Just a minor suggestion but looks good

@DeepikaGhodki DeepikaGhodki requested a review from jjnesbitt July 22, 2022 08:00
Copy link
Member

@mvandenburgh mvandenburgh left a comment

Choose a reason for hiding this comment

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

Looks good, just one issue I noticed when testing it out

@jjnesbitt
Copy link
Member

LGTM as far as the changes I asked for, but I'll defer approval to @mvandenburgh since he's more recently asked for changes.

Copy link
Member

@mvandenburgh mvandenburgh left a comment

Choose a reason for hiding this comment

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

LGTM

@DeepikaGhodki DeepikaGhodki merged commit 7d1805f into master Aug 1, 2022
@DeepikaGhodki DeepikaGhodki deleted the move-between-pages branch August 1, 2022 20:07
@dandibot
Copy link
Member

dandibot commented Aug 3, 2022

🚀 PR was released in v0.2.42 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants