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

Use and prefer view config components by default over the default view config partials #2523

Merged
merged 3 commits into from Nov 17, 2021

Conversation

cbeer
Copy link
Member

@cbeer cbeer commented Nov 16, 2021

Now that we're enabling and configuring view components enabled by default, we can stop configuring partials by default. Partial configuration is still available and will append the rendered content to the document.

@cbeer cbeer marked this pull request as draft November 16, 2021 18:40
@cbeer cbeer force-pushed the view-partials branch 2 times, most recently from 65b6a09 to d962fef Compare November 16, 2021 18:59
@dkinzer
Copy link
Member

dkinzer commented Nov 16, 2021

I've been playing around with upgrading one of our instances to latest changes and it fails with that, but with this branch I'm able to upgrade without issues.

@cbeer
Copy link
Member Author

cbeer commented Nov 16, 2021

Hm. I wouldn't expect that #2458 would have changed anything. Can you provide more detail about what the issue was? I do think the repurposed partial configuration here will make migration easier (but are difficult to make backwards compatible), so maybe that's it?

@dkinzer
Copy link
Member

dkinzer commented Nov 17, 2021

@cbeer the error I getting at fd33c74

is

NoMethodError in Catalog#show

Showing /Users/dkinzer/projects/funnel_cake/app/views/catalog/_show.html.erb where line #1 raised:

undefined method `show_presenter' for #<#<Class:0x00007fa748452a58>:0x00007fa748405af0>
Extracted source (around line #1):
1 2 3 4 5 6 <% doc_presenter = show_presenter(document) %> <%# default partial to display solr document fields in catalog show view -%>

Which happens when I try to look at a document. If I switch to this branch the issue is resolved.

@dkinzer
Copy link
Member

dkinzer commented Nov 17, 2021

Also, we are overriding that template that is failing with this

<% doc_presenter = show_presenter(document) %>


<%# default partial to display solr document fields in catalog show view -%>
<div class="row">
  <div class="col-12">
    <dl class="row dl-invert document-metadata">
      <% doc_presenter.fields_to_render.each do |field_name, field| -%>
        <dt class="blacklight-<%= field_name.parameterize %> col-md-3"><%= render_document_show_field_label document, field: field_name %></dt>
        <dd class="align-self-center blacklight-<%= field_name.parameterize %> col-md-9"><%= doc_presenter.field_value field %></dd>
      <% end -%>
    </dl>
  </div>
  <div class="col-12">
    <dl class="row dl-invert document-metadata">
      <dt class="align-self-center col-md-3"><%= staff_view_link(document.id) %></dt>
    </dl>
  </div>
</div>

@cbeer cbeer marked this pull request as ready for review November 17, 2021 20:04
@jcoyne
Copy link
Member

jcoyne commented Nov 17, 2021

@dkinzer show_presenter was removed from the 8.x (main) branch. It was deprecated https://github.com/projectblacklight/blacklight/blob/release-7.x/app/helpers/blacklight/blacklight_helper_behavior.rb#L338

@jcoyne jcoyne merged commit 922d871 into main Nov 17, 2021
@jcoyne jcoyne deleted the view-partials branch November 17, 2021 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants