Sidebar display fixes #1442
Sidebar display fixes #1442
Conversation
| @@ -0,0 +1,23 @@ | |||
| <div class="card relations relationship-<%= relationship_type.downcase %>"> | |||
| <div class="card-header"> | |||
| <h2 class="h4"><%= t("#{rel_type_info.label}") %></h2> | |||
There was a problem hiding this comment.
| <h2 class="h4"><%= t("#{rel_type_info.label}") %></h2> | |
| <h2 class="h4"><%= t(rel_type_info.label) %></h2> |
| ] | ||
| end | ||
|
|
||
| # Second section of the sidebar that displays below the main one | ||
| def secondary_components |
There was a problem hiding this comment.
Can we rename this to secondary_components_or_partials, or perhaps we can make RelationsContainerComponent? so that these are all components.
There was a problem hiding this comment.
Can I do that in Geoblacklight as part of a refactor to RelationsComponent? It seems like that's most of what you would like to see anyway. I can change the name of the method for now, if you like.
|
|
||
| # Determine if there's actually anything to show by rendering everything | ||
| def filter_components(components) | ||
| components.map { |component| render component, document: }.uniq.compact_blank |
There was a problem hiding this comment.
Not sure – I just extracted this from the template into a method. Doesn't seem like we could conceivably duplicate a component, so I'll remove it.
| </div> | ||
|
|
||
| <ul class="list-group list-group-flush"> | ||
| <% relationship_type_results['docs'][0..2].map { |hash| SolrDocument.new(hash) }.each do |doc| %> |
There was a problem hiding this comment.
Can you move this code out of the template and into a method in the class?
| <% end %> | ||
| </li> | ||
| <% end %> | ||
| <% unless (relationship_type_results['numFound'].to_i <= 3) %> |
There was a problem hiding this comment.
Can you move relationship_type_results['numFound'] to a method on the class? It can be used on line 18 too.
| <% end %> | ||
| <% unless (relationship_type_results['numFound'].to_i <= 3) %> | ||
| <li class="list-group-item border-bottom-0"> | ||
| <%= link_to search_catalog_path({f: {"#{Settings.RELATIONSHIPS_SHOWN.public_send(relationship_type).field}" => [relations.link_id]}}) do %> |
There was a problem hiding this comment.
How about moving this line to a method too?
app/views/relation/index.html.erb
Outdated
| @@ -0,0 +1,3 @@ | |||
| <%- Settings.RELATIONSHIPS_SHOWN.each do |relationship_type, rel_type_info| %> | |||
There was a problem hiding this comment.
Where is this partial called from?
|
@jcoyne just so you're aware – |
thatbudakguy
force-pushed
the
sidebar-display
branch
from
d8f6d02 to
bd8dc5d
Compare
|
I went ahead and removed the commit in this PR dealing with |
thatbudakguy
force-pushed
the
sidebar-display
branch
from
bd8dc5d to
6cebd36
Compare
| <% if filtered_components.any? %> | ||
| <div class="bg-fog-light component-container rounded mt-4"> | ||
| <%= filtered_components.join(separator).html_safe %> |
There was a problem hiding this comment.
Can we use safe_join here?
| <%= filtered_components.join(separator).html_safe %> | |
| <%= safe_join(filtered_components, separator) %> |
| <hr style="border-top: dotted 1px;" /> | ||
| <% end %> | ||
| <%= tools %> | ||
| <%= filtered_components.join(separator).html_safe %> |
There was a problem hiding this comment.
| <%= filtered_components.join(separator).html_safe %> | |
| <%= safe_join filtered_components, separator %> |
This refactors the sidebar component to make it easier to determine what is being rendered in each of the two sections and to render it without accidentally duplicating the divider elements. It also ensures that items that should be grouped together in the second section are displayed in a single box, as noted in #1437. Fixes #1437
thatbudakguy
force-pushed
the
sidebar-display
branch
from
6cebd36 to
5fb83a5
Compare
|
Thanks for the tip re: |
Before:
After:
Part of #1437