From 3b1e1fd69646f9bd60d63ac9d5d403a5eea59761 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Fri, 14 Nov 2025 09:00:24 -0500 Subject: [PATCH 1/2] Update pagination buttons to include results count Why are these changes being introduced: * UX design includes showing the number of results that will be shown when clicking the Next or Previous pagination buttons. * Previous page should always be the same number of results as per_page so it is simpler to say "Previous 20 results" rather than trying to calculate how many results remain before the start index. * Next page should show the remaining results if fewer than per_page results remain. * Also introduces a disabled state via a span for the next and previous buttons when there are no more results in that direction. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-184 How does this address that need: * Mathed out the new pagination button text requirements * Updated PaginationHelper methods to calculate remaining results * Updated PaginationHelper methods to render disabled span when no more results in that direction --- app/helpers/pagination_helper.rb | 58 +++++++++++++++++----- app/models/analyzer.rb | 7 +-- app/views/search/_pagination.html.erb | 19 ++------ test/helpers/pagination_helper_test.rb | 66 ++++++++++++++++++-------- 4 files changed, 98 insertions(+), 52 deletions(-) diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb index f31bbca9..060463d0 100644 --- a/app/helpers/pagination_helper.rb +++ b/app/helpers/pagination_helper.rb @@ -7,10 +7,16 @@ def first_url(query_params) # Preserve the active tab in pagination URLs. params_copy[:tab] = @active_tab if @active_tab.present? - link_to results_path(params_copy), 'aria-label': 'First page', - data: { turbo_frame: 'search-results', turbo_action: 'advance' }, - rel: 'nofollow' do - '«« First'.html_safe + # First page gets a disabled link in a span + # Check the original, not copied params to determine if we need a disabled link + if query_params[:page].blank? || query_params[:page].to_i == 1 + 'First'.html_safe + else + link_to results_path(params_copy), 'aria-label': 'First', + data: { turbo_frame: 'search-results', turbo_action: 'advance' }, + rel: 'nofollow' do + 'First'.html_safe + end end end @@ -22,13 +28,32 @@ def next_url(query_params) # Preserve the active tab in pagination URLs. params_copy[:tab] = @active_tab if @active_tab.present? - link_to results_path(params_copy), 'aria-label': 'Next page', - data: { turbo_frame: 'search-results', turbo_action: 'advance' }, - rel: 'nofollow' do - 'Next »'.html_safe + if remaining_results <= 0 + "#{next_page_label}".html_safe + else + link_to results_path(params_copy), 'aria-label': next_page_label, + data: { turbo_frame: 'search-results', turbo_action: 'advance' }, + rel: 'nofollow' do + next_page_label.html_safe + end end end + # Calculate how many results remain after the current end index + def remaining_results + @pagination[:hits] - @pagination[:end] + end + + def next_page_label + label = if (@pagination[:end] + @pagination[:per_page]) < @pagination[:hits] + @pagination[:per_page] + else + remaining_results + end + + "Next #{label} results" + end + def prev_url(query_params) # Work with a copy to avoid mutating the original enhanced_query. params_copy = query_params.dup @@ -37,10 +62,19 @@ def prev_url(query_params) # Preserve the active tab in pagination URLs. params_copy[:tab] = @active_tab if @active_tab.present? - link_to results_path(params_copy), 'aria-label': 'Previous page', - data: { turbo_frame: 'search-results', turbo_action: 'advance' }, - rel: 'nofollow' do - '« Previous'.html_safe + # First page gets a disabled link in a span + if query_params[:page].blank? || query_params[:page].to_i == 1 + "#{prev_page_label}".html_safe + else + link_to results_path(params_copy), 'aria-label': prev_page_label, + data: { turbo_frame: 'search-results', turbo_action: 'advance' }, + rel: 'nofollow' do + prev_page_label.html_safe + end end end + + def prev_page_label + 'Previous 20 results' + end end diff --git a/app/models/analyzer.rb b/app/models/analyzer.rb index 1c92b8c7..f5d2a774 100644 --- a/app/models/analyzer.rb +++ b/app/models/analyzer.rb @@ -15,9 +15,10 @@ def initialize(enhanced_query, response, source) @pagination[:start] = ((enhanced_query[:page] - 1) * RESULTS_PER_PAGE) + 1 @pagination[:end] = [enhanced_query[:page] * RESULTS_PER_PAGE, @pagination[:hits]].min @pagination[:prev] = enhanced_query[:page] - 1 if enhanced_query[:page] > 1 - - next_page_num = next_page(enhanced_query[:page], @pagination[:hits]) - @pagination[:next] = next_page_num if next_page_num + @pagination[:next] = next_page(enhanced_query[:page], @pagination[:hits]) if next_page( + enhanced_query[:page], @pagination[:hits] + ) + @pagination[:per_page] = RESULTS_PER_PAGE end private diff --git a/app/views/search/_pagination.html.erb b/app/views/search/_pagination.html.erb index 20b8b29f..0b6d8878 100644 --- a/app/views/search/_pagination.html.erb +++ b/app/views/search/_pagination.html.erb @@ -2,26 +2,13 @@ diff --git a/test/helpers/pagination_helper_test.rb b/test/helpers/pagination_helper_test.rb index 8e6266c8..592e90e0 100644 --- a/test/helpers/pagination_helper_test.rb +++ b/test/helpers/pagination_helper_test.rb @@ -4,69 +4,93 @@ class PaginationHelperTest < ActionView::TestCase include PaginationHelper test 'Next links for basic search' do - @pagination = { next: 12 } - query_params = { q: 'popcorn' } + @pagination = { prev: 11, next: 13, per_page: 20, hits: 1000, end: 260 } + query_params = { q: 'popcorn', page: 12 } assert_equal( - 'Next »', next_url(query_params) + 'Next 20 results', next_url(query_params) ) end test 'Next links for advanced search' do - @pagination = { next: 12 } + @pagination = { prev: 11, next: 13, per_page: 20, hits: 1000, end: 260 } query_params = { q: 'popcorn', title: 'titles are cool', contributors: 'yawn' } assert_equal( - 'Next »', + 'Next 20 results', next_url(query_params) ) end test 'Previous links for basic search' do - @pagination = { prev: 11 } - query_params = { q: 'popcorn' } + @pagination = { prev: 11, next: 13, per_page: 20, hits: 1000, end: 260 } + query_params = { q: 'popcorn', page: 12 } assert_equal( - '« Previous', prev_url(query_params) + 'Previous 20 results', prev_url(query_params) ) end test 'Previous links for advanced search' do - @pagination = { prev: 11 } - query_params = { q: 'popcorn', title: 'titles are cool', contributors: 'yawn' } + @pagination = { prev: 11, next: 13, per_page: 20, hits: 1000, end: 260 } + query_params = { q: 'popcorn', title: 'titles are cool', contributors: 'yawn', page: 12 } assert_equal( - '« Previous', + 'Previous 20 results', prev_url(query_params) ) end test 'Next links preserve active tab' do - @pagination = { next: 12 } + @pagination = { prev: 11, next: 13, per_page: 20, hits: 1000, end: 260 } @active_tab = 'primo' - query_params = { q: 'popcorn' } + query_params = { q: 'popcorn', page: 12 } assert_equal( - 'Next »', next_url(query_params) + 'Next 20 results', next_url(query_params) ) end test 'Previous links preserve active tab' do - @pagination = { prev: 11 } + @pagination = { prev: 11, next: 13, per_page: 20, hits: 1000, end: 260 } @active_tab = 'timdex' - query_params = { q: 'popcorn' } + query_params = { q: 'popcorn', page: 12 } assert_equal( - '« Previous', prev_url(query_params) + 'Previous 20 results', prev_url(query_params) ) end - test 'First links for basic search' do + test 'First links for initial basic search is disabled' do query_params = { q: 'popcorn' } assert_equal( - '«« First', first_url(query_params) + 'First', first_url(query_params) ) end test 'First links preserve active tab' do @active_tab = 'primo' - query_params = { q: 'popcorn' } + query_params = { q: 'popcorn', page: 5 } + assert_equal( + 'First', first_url(query_params) + ) + end + + test 'Next link for penultimate page is correct' do + @pagination = { next: 6, per_page: 20, hits: 102, end: 100 } + query_params = { q: 'popcorn', page: 5 } + assert_equal( + 'Next 2 results', next_url(query_params) + ) + end + + test 'Next link for full last page is disabled' do + @pagination = { next: 6, per_page: 20, hits: 100, end: 100 } + query_params = { q: 'popcorn', page: 5 } + assert_equal( + "Next 0 results", next_url(query_params) + ) + end + + test 'Previous link for first page is disabled' do + @pagination = { prev: nil, per_page: 20, hits: 100, end: 20 } + query_params = { q: 'popcorn', page: 1 } assert_equal( - '«« First', first_url(query_params) + "Previous 20 results", prev_url(query_params) ) end end From 2207fac29bbd7eae8ecf9fbbdfd0198a4e3ff34b Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Fri, 14 Nov 2025 11:52:01 -0500 Subject: [PATCH 2/2] Previous page uses per_page value --- app/helpers/pagination_helper.rb | 16 +++++++++++----- test/helpers/pagination_helper_test.rb | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb index 060463d0..bb6f6ab5 100644 --- a/app/helpers/pagination_helper.rb +++ b/app/helpers/pagination_helper.rb @@ -64,17 +64,23 @@ def prev_url(query_params) # First page gets a disabled link in a span if query_params[:page].blank? || query_params[:page].to_i == 1 - "#{prev_page_label}".html_safe + "#{prev_page_label(query_params[:page].to_i || 1)}".html_safe else - link_to results_path(params_copy), 'aria-label': prev_page_label, + link_to results_path(params_copy), 'aria-label': prev_page_label(query_params[:page].to_i || 1), data: { turbo_frame: 'search-results', turbo_action: 'advance' }, rel: 'nofollow' do - prev_page_label.html_safe + prev_page_label(query_params[:page].to_i || 1).html_safe end end end - def prev_page_label - 'Previous 20 results' + def prev_page_label(current_page = 1) + label = if current_page == 1 + 0 + else + @pagination[:per_page] + end + + "Previous #{label} results" end end diff --git a/test/helpers/pagination_helper_test.rb b/test/helpers/pagination_helper_test.rb index 592e90e0..d7275396 100644 --- a/test/helpers/pagination_helper_test.rb +++ b/test/helpers/pagination_helper_test.rb @@ -90,7 +90,7 @@ class PaginationHelperTest < ActionView::TestCase @pagination = { prev: nil, per_page: 20, hits: 100, end: 20 } query_params = { q: 'popcorn', page: 1 } assert_equal( - "Previous 20 results", prev_url(query_params) + "Previous 0 results", prev_url(query_params) ) end end