-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Turbo Frames pagination bug #257
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 18945735539Details
💛 - Coveralls |
9f49e9e to
df9362e
Compare
JPrevost
reviewed
Oct 30, 2025
JPrevost
approved these changes
Oct 30, 2025
Why these changes are being introduced: Navigation past page 2 in search results is broken due to Turbo Frame state management issues. Pagination controls were outside the Turbo Frame, causing full page reloads that lost tab context and broke the pagination flow. Additionally, Primo API has practical limits around 960 records (or thereabouts), despite claiming millions of results and a recommended offset limit of 2,000 and a hardcoded limit of 5,000. This leads to inconsistent behavior at high page counts (errors, timeouts, etc). Relevant ticket(s): - [USE-95](https://mitlibraries.atlassian.net/browse/USE-95) - [USE-110](https://mitlibraries.atlassian.net/browse/USE-110) How this addresses that need: - Moves pagination controls inside the `search-results` Turbo Frame to maintain state during navigation. - Adds `data-turbo-action='advance'` to pagination links for proper URL updates and browser history management. - Adds `rel='nofollow'` to pagination links to prevent bot crawling. - Set PRIMO_MAX_OFFSET at 960 based on real-world API testing. - Implement continuation page for Primo results beyond pagination limits. - Consolidates result count display logic to show `10,000+` for all sources when appropriate. - Adds a `First` affordance for users to return quickly to the first page. Side effects of this change: - Pagination now uses AJAX updates instead of full page reloads, providing faster navigation. (This is marginal given how slow the Primo API is.) - Users will see a "Continue in Search Our Collections" page when reaching Primo's practical pagination limits. It's notable that Primo UI has the same bug as the Search API, so they will continue to experience frustration at higher page counts regardless of which interface they use. - The URL updates provided by `data-turbo-action='advance'` also fixes USE-110 (ensuring consistent browser state). - Some autoformatting applied by VSCode around integers (e.g., 10000 becomes 10_000). - I have serious concerns about the Primo Search API performance. It is so slow that it times out occasionally after 60 seconds. This makes pagination an inherently poor user experience, as the problem seems to get worse at higher page numbers. I strongly urge us to reconsider making Primo the default tab. If that is not an option, we should evaluate creative workarounds. (E.g., perhaps a higher per-page count would dissuade users from using pagination.) Caching alone is unlikely to help in this case.
e75f523 to
9da1f33
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why these changes are being introduced:
Navigation past page 2 in search results is broken due to Turbo Frame state management issues. Pagination controls were outside the Turbo Frame, causing full page reloads that lost tab context and broke the pagination flow.
Additionally, Primo API has practical limits around 960 records (or thereabouts), despite claiming millions of results and a recommended offset limit of 2,000 and a hardcoded limit of 5,000. This leads to inconsistent behavior at high page counts (errors, timeouts, etc).
Relevant ticket(s):
How this addresses that need:
data-turbo-action='advance'to pagination links for proper URL updates and browser history management.Side effects of this change:
data-turbo-action='advance'also fixes USE-110 (ensuring consistent browser state).Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
Please test USE and GeoData.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing