diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb index f31bbca..bb6f6ab 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,25 @@ 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(query_params[:page].to_i || 1)}".html_safe + else + 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(query_params[:page].to_i || 1).html_safe + end end end + + 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/app/models/analyzer.rb b/app/models/analyzer.rb index 1c92b8c..f5d2a77 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 20b8b29..0b6d887 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 8e6266c..d727539 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 0 results", prev_url(query_params) ) end end