-
Notifications
You must be signed in to change notification settings - Fork 359
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
fix(ui): browser back button on "all"-pages #2005
Conversation
By directly linking to the URL with query parameters we automatically load the first page. This avoids reloading the first page via `loadFirstPage()` on in `consumer-orgs/apis.ts` which would create two browser history entries. Keeping `loadFirstPage()` is still useful to display the first results after reloading the page or after resetting the search. Closes: #1181
@@ -71,7 +71,7 @@ | |||
</a> | |||
</li> | |||
<li class="list-group-item" id="apiman-sidebar-apis-consume" data-target="#apis-all_apis-tertiary"> | |||
<a href="{{ pluginName }}/browse/apis"> | |||
<a href="{{ pluginName }}/browse/apis?q=*&cp=1&ps=12"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the cp
and ps
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current page and page size I guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, so the browse/consume page uses the search endpoint.
So to avoid displaying an empty page on first page load we fetch the first page of a wildcard search (SearchBean).
q = querry
cp = currentPage
ps = pageSize
This is a nice default which does not display to much data and plays nice with our 4x3 card layout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you guys add that?/ I don't remember that being the behaviour Apiman had historically (limiting page size to 12)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is the case since a while, apiman 2.x behaves the same.
You can change the page size in the UI if there are more entries to 25, 50 and 100 I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let's just leave it for now. Perhaps needs to be a bit more flexible and signpost that you can search more easily.
Sorry forgot one file will push a fixup in a minute |
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
By directly linking to the URL with query parameters we automatically load the first page. This avoids reloading the first page via
loadFirstPage()
inconsumer-orgs/apis.ts
which would create two browser history entries.Keeping
loadFirstPage()
is still useful to display the first results after reloading the page or after resetting the search.Closes: #1181