-
Notifications
You must be signed in to change notification settings - Fork 354
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 first/last navigation. #3404
Fix first/last navigation. #3404
Conversation
Separates record page first/last navigation setting from backend support for first/last navigation by improved naming of the variable and method. Also adds flag about support for first/last navigation in results to the Options class so that it works properly with JS results and doesn't need to be set in a backend-specific template.
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 checked out this branch and tested it in all three themes. The desired functionality is in place for the results page paginator: the first/last numbers in brackets are present at all the times they're supposed to be present. Looks good!
However, I'm finding that the paginators in Checked Out Items and Loan History now are displaying disabled links, and are showing the words "First" and "Last" inside the "Previous" and "Next" arrow/buttons instead of showing [1] and [4] outside those arrows:
I think we want the paginators in the My Account area to have the same appearance as the results page paginator, if possible.
Thanks, @sturkel89! I believe that the "My Account" improvements you refer to are part of #3391 but aren't included here. Once this is approved and merged over there, I think we'll have the best of all worlds. |
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.
Looks good to me; thanks, @EreMaijala!
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.
Changing my review to Approve! :)
Separates record page first/last navigation setting from backend support for first/last navigation by improved naming of the variable and method. Also adds flag about support for first/last navigation in results to the Options class so that it works properly with JS results and doesn't need to be set in a backend-specific template.
Spin-off from #3391.
TODO
supportsFirstLastNavigation()
and removal offirstlastNavigation
property.