Skip to content
This repository has been archived by the owner on Mar 24, 2020. It is now read-only.

Add generic thumbnail and restricted view info to search objects view #442

Merged
merged 2 commits into from
May 2, 2018

Conversation

VivianChu
Copy link
Member

Fixes #411 ; refs #411

Add grey generic thumbnails and restricted view access info for objects of metadata data-only visibility collections.

@ucsdlib/developers - please review

<%= link_to url, :class => 'dams-search-thumbnail-link' do %>
<%- if !restrictedNotice.nil? && rtn = restricted_object_url%>
<div class="document-thumbnail dams-search-thumbnail">
<%= rtn %>
</div>
<%- elsif access_notice && rtn = restricted_object_url%>
Copy link
Member

Choose a reason for hiding this comment

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

How hard would it be to have a helper function that returns the URL we want for the link? We've got 3 branches here that all render the same div with the same classes and then the link. I'm just wondering if we could pull the logic out so the link_to is basically just the div with the returned value from a method that sorts out the correct URL. If that's a pain, then no worries! Just a thought.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 66.201% when pulling 6286d08 on feature/411_items_view into 2b63101 on develop.

@VivianChu
Copy link
Member Author

@mcritchlow - I created a helper method for the url. Could you review them again? Thanks

Copy link
Member

@mcritchlow mcritchlow left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hweng
Copy link
Contributor

hweng commented May 2, 2018

👍

@hweng hweng merged commit faf921a into develop May 2, 2018
@hweng hweng deleted the feature/411_items_view branch May 2, 2018 18:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants