Skip to content

Conversation

@frrrances
Copy link
Contributor

No description provided.

@gravesm gravesm temporarily deployed to mit-bento-staging-pr-134 February 22, 2017 13:55 Inactive
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f7eeda1 on frances/temp/hide-status into 6ce9c4c on master.

@frrrances frrrances merged commit 2e6eab3 into master Feb 22, 2017
@frrrances frrrances deleted the frances/temp/hide-status branch February 22, 2017 14:24
<ul class="list-local-locations">
<% result.location.each do |location| %>
<li class="result-local-location">
<%= location[0] %>: <%= location[1] %> (not live)
Copy link
Member

Choose a reason for hiding this comment

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

This line shows us the cached status from EDS. It is displayed if anything goes wrong when requesting the realtime information. The text should reflect that it is not realtime information. I don't particularly care what exactly we say, but we should make it clear it is not realtime.

<li class=""><a href="">Sed diam nonumy eirmod</a></li>
<li class=""><a href="">Tempor invidunt ut labore</a></li>
<li class=""><a href="">Dolore magna aliquyam</a></li>
<li class="http://libraries.mit.edu/bartonplus"><a href="library.mit.edu">Search in BartonPlus</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

This css class is probably not what you intended ;)

Also, is there a reason we don't consistently use https when linking to libraries.mit.edu?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... and this is why we have code reviews before merge. 😞 and yes, we should be linking with https. I'll make any changes in the next PR.

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.

5 participants