Skip to content
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

Copy SearchContextComponent to ServerItemPaginationComponent to ease BL8 upgrade path #3421

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div class='pagination-search-widgets'>

<div class="page-links">
<%= link_to_previous_document %> |

<%= item_page_entry_info %> |

<%= link_to_next_document %>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true

module Blacklight
module SearchContext
class ServerItemPaginationComponent < Blacklight::Component
with_collection_parameter :search_context

def initialize(search_context:, search_session:, current_document:)
@search_context = search_context
@search_session = search_session
@current_document_id = current_document.id
end

def render?
@search_context.present? && (@search_context[:prev] || @search_context[:next])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blacklight 8 has this as:

Suggested change
@search_context.present? && (@search_context[:prev] || @search_context[:next])
@search_context.present? && (@search_context[:prev] || @search_context[:next] || total.positive?) && (@search_session['document_id'] == @current_document_id)

Should this match, or is there a reason this is different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that version and it wasn't compatible with Blacklight 7.

My hope with this PR is that we will be able to add a component to replace our custom pagination partial that will work with both Blacklight 7 and Blacklight 8 without significant revisions when upgrading between versions.

end

def item_page_entry_info
Deprecation.silence(Blacklight::CatalogHelperBehavior) do
helpers.item_page_entry_info
end
end

def link_to_previous_document(document = nil, *args, **kwargs)
Deprecation.silence(Blacklight::UrlHelperBehavior) do
helpers.link_to_previous_document(document || @search_context[:prev], *args, **kwargs)
end
end

def link_to_next_document(document = nil, *args, **kwargs)
Deprecation.silence(Blacklight::UrlHelperBehavior) do
helpers.link_to_next_document(document || @search_context[:next], *args, **kwargs)
end
end
end
end
end
1 change: 1 addition & 0 deletions app/components/blacklight/search_context_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class SearchContextComponent < Blacklight::Component
with_collection_parameter :search_context

def initialize(search_context:, search_session:)
Blacklight.deprecation.warn("Blacklight::SearchContextComponent is deprecated and will be moved to Blacklight::SearchContext::ServerItemPaginationComponent in Blacklight 8.0.0")
@search_context = search_context
@search_session = search_session
end
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/blacklight/catalog_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def item_page_entry_info
total: number_with_delimiter(search_session['total']),
count: search_session['total'].to_i).html_safe
end
deprecation_deprecate item_page_entry_info: 'Use Blacklight::SearchContextComponent methods instead'
deprecation_deprecate item_page_entry_info: 'Use Blacklight::SearchContext::ServerItemPaginationComponent methods instead'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻


##
# Look up search field user-displayable label
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/blacklight/url_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def link_to_previous_document(previous_document, classes: 'previous', **addl_lin
tag.span raw(t('views.pagination.previous')), class: 'previous'
end
end
deprecation_deprecate link_to_previous_document: 'Moving to Blacklight::SearchContextComponent'
deprecation_deprecate link_to_previous_document: 'Moving to Blacklight::SearchContext::ServerItemPaginationComponent'

##
# Link to the next document in the current search context
Expand All @@ -69,7 +69,7 @@ def link_to_next_document(next_document, classes: 'next', **addl_link_opts)
tag.span raw(t('views.pagination.next')), class: 'next'
end
end
deprecation_deprecate link_to_previous_document: 'Moving to Blacklight::SearchContextComponent'
deprecation_deprecate link_to_previous_document: 'Moving to Blacklight::SearchContext::ServerItemPaginationComponent'

##
# Attributes for a link that gives a URL we can use to track clicks for the current search session
Expand Down
4 changes: 2 additions & 2 deletions app/views/catalog/_previous_next_doc.html.erb
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<% Deprecation.warn(self, 'The partial _previous_next_doc.html.erb will be removed in 8.0. Render Blacklight::SearchContextComponent instead.') %>
<%= render(Blacklight::SearchContextComponent.new(search_context: @search_context, search_session: search_session)) %>
<% Deprecation.warn(self, 'The partial _previous_next_doc.html.erb will be removed in 8.0. Render Blacklight::SearchContext::ServerItemPaginationComponent instead.') %>
<%= render(Blacklight::SearchContext::ServerItemPaginationComponent.new(search_context: @search_context, search_session: search_session, current_document: @current_document)) %>
2 changes: 1 addition & 1 deletion app/views/catalog/_show_main_content.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= render(Blacklight::SearchContextComponent.new(search_context: @search_context, search_session: search_session)) if search_session['document_id'] == @document.id %>
<%= render(Blacklight::SearchContext::ServerItemPaginationComponent.new(search_context: @search_context, search_session: search_session, current_document: @document)) if search_session['document_id'] == @document.id %>

<% @page_title = t('blacklight.search.show.title', document_title: Deprecation.silence(Blacklight::BlacklightHelperBehavior) { document_show_html_title }, application_name: application_name).html_safe %>
<% content_for(:head) { render_link_rel_alternates } %>
Expand Down
2 changes: 2 additions & 0 deletions spec/views/catalog/_previous_next_doc.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@

it "without next or previous does not render content" do
assign(:search_context, {})
assign(:current_document, SolrDocument.new(id: 9))
render
expect(rendered).not_to have_selector ".pagination-search-widgets"
end

it "with next or previous does render content" do
assign(:search_context, next: 'foo', prev: 'bar')
assign(:current_document, SolrDocument.new(id: 9))
allow(view).to receive(:link_to_previous_document).and_return('')
allow(view).to receive(:item_page_entry_info).and_return('')
allow(view).to receive(:link_to_next_document).and_return('')
Expand Down
Loading