TD-1337 restore local filter #436
Conversation
zinc-glaze
commented
Sep 27, 2024
•
zinc-glaze
commented
- Deprecates 'search_sidebar' partial (replaced w/ componenent in BL8)
- Overrides 'search/sidebar_component' to render 'local_filter' instead
- Modifies 'local_filter' partial to include markup previously rendered by 'search_sidebar' partial
- fixes rubocop errors
| @@ -1,3 +1,4 @@ | |||
| <% # this partial is deprecated with BL8. catalog/index now renders the sidebar component %> | |||
There was a problem hiding this comment.
We should just delete this partial entirely as part of this PR rather than keep it around unused in the codebase.
app/views/catalog/index.html.erb
Outdated
| @@ -0,0 +1,26 @@ | |||
| <% # TRLN override of BL8 catalog/index view %> | |||
There was a problem hiding this comment.
This does indeed work, but overriding the entire BL core index template comes at a cost. It's probably better in this case to just override/extend BL's view for the sidebar component specifically (https://github.com/projectblacklight/blacklight/blob/release-8.x/app/components/blacklight/search/sidebar_component.html.erb). E.g., if you make a local copy of only that file, then put this at the top, it'd work:
<%= render 'catalog/local_filter',
local_button_id: 'toggle-local-btn-top',
trln_button_id: 'toggle-trln-btn-top' %>
It's a bit subjective, so I think worth team weigh-in on how we want to approach this kind of refactor for BL8.
There was a problem hiding this comment.
Okay, sounds good. I tried to create a new "local_filter" component to replace that partial, but found that the methods and variables that were available to the view partial were undefined when used in the component. You're saying to remove the "local_filter" partial and move that markup/logic into an override of the new sidebar component? I think it might be a little more complicated than that as the new sidebar component is not a 1-to-1 replacement for the old sidebar partial.
There was a problem hiding this comment.
I would keep the _local_filter.html.erb partial how you have it. Copy the https://github.com/projectblacklight/blacklight/blob/release-8.x/app/components/blacklight/search/sidebar_component.html.erb file into this engine at the same path. Prepend that snippet to the top, annotated with comments like you've done elsewhere.
You probably don't need to pass search_state to the partial. If you do, the component can still call that method but with helpers.search_state, i.e.:
<%= render 'catalog/local_filter',
search_state: helpers.search_state,
local_button_id: 'toggle-local-btn-top',
trln_button_id: 'toggle-trln-btn-top' %>
There was a problem hiding this comment.
I found that i had to also include the 'sidebar_component.rb' to get the override to work, as well as updating the configuration to use the override.
…ecate 'search_sidebar' partial per BL8; modify 'local_filter' partial to duplicate deprecated 'search_sidebar'
…ide; fix markup error
zinc-glaze
force-pushed
the
TD-1337-restore-local-filter
branch
from
de4938b to
b73642b
Compare
| module TrlnArgon | ||
| module Search | ||
| class SidebarComponent < Blacklight::Search::SidebarComponent | ||
| def initialize(blacklight_config:, response:, view_config:) |
There was a problem hiding this comment.
I would remove lines 6-15 here, which will spare us from having to reconcile this code frequently against upstream Blacklight to make sure it matches.
