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

Better Highlights Need for Archive-It Credentials, Resolves #293 #295

Merged
merged 3 commits into from May 9, 2019

Conversation

Projects
None yet
4 participants
@ianmilligan1
Copy link
Member

commented May 9, 2019

GitHub issue(s):

#293

What does this Pull Request do?

In #293, we noted that we needed to highlight the need for an Archive-It account to use the Cloud. While that may change down the road, right now we have a lot of folks who sign up for the Cloud but then realize they need an Archive-It account and are a bit confused or disappointed.

This PR does two main things:

  • adds a warning bar on the main page, laying out the situation for new users and asking them to read the FAQ;
  • Notes the Archive-It connection explicitly in the "Project Overview" section of the "about" page. While that information is available if you scroll down the page, this hopefully does a better job of highlighting this. The FAQ already makes the situation clear up front.

How should this be tested?

TravisCI should turn green. Please review for language suggestions and aesthetic appeal.

Screenshots:

Screenshots added for convenience.

Here's the new home page:
Screen Shot 2019-05-09 at 10 42 16 AM

And here's the added content to the "about" page (red arrow is my screenshot annotation):
Screen Shot 2019-05-09 at 11 17 24 AM

Interested parties

@ruebot @SamFritz

@ianmilligan1 ianmilligan1 requested review from ruebot and SamFritz May 9, 2019

@ruebot

ruebot approved these changes May 9, 2019

@@ -1,4 +1,10 @@
<% cache do %>
<div class="alert alert-warning alert-dismissible fade show" role="alert">
<strong>New user?</strong> You currently need Archive-It credentials to use our service. Please read our <%= link_to('FAQ', '/faq', target: '_blank') %>.

This comment has been minimized.

Copy link
@ruebot

ruebot May 9, 2019

Member

Let's link to Archive-It too.

This comment has been minimized.

Copy link
@ianmilligan1

ianmilligan1 May 9, 2019

Author Member

Good call!

@codecov-io

This comment has been minimized.

Copy link

commented May 9, 2019

Codecov Report

Merging #295 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #295   +/-   ##
=======================================
  Coverage   89.33%   89.33%           
=======================================
  Files          35       35           
  Lines         619      619           
=======================================
  Hits          553      553           
  Misses         66       66

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fef483e...239787c. Read the comment docs.

@ruebot ruebot merged commit b4f5758 into master May 9, 2019

3 checks passed

codecov/patch Coverage not affected when comparing fef483e...239787c
Details
codecov/project 89.33% remains the same compared to fef483e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ruebot ruebot deleted the issue-293 branch May 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.