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

Hide banner for third party users #628

Closed
wants to merge 3 commits into from

Conversation

robertknight
Copy link
Member

Depends on #627

This hides the "To annotate, create a free account or log in" banner on pages that use third-party accounts.

This is a little bit of cleanup before fixing an issue where the banner
shows inappropriate text on publisher sites using third party accounts.
This will make it easier to extend the logic depending on whether the
site uses Hypothesis accounts or not.
…rd party accounts

This banner is problematic on pages using publisher-managed accounts or
a third party annotation service because:

 - The "Create account" link currently goes to hypothes.is/signup since
   we do not yet have the ability to customize the output of
   `/api/links` depending on the authority.

 - Accounts may not be free to create and the user might not be able to
   create one themselves.

Resolve this by just hiding the banner in this case.
@codecov
Copy link

codecov bot commented Dec 7, 2017

Codecov Report

Merging #628 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #628      +/-   ##
==========================================
+ Coverage   90.98%   90.99%   +0.01%     
==========================================
  Files         135      135              
  Lines        5368     5375       +7     
  Branches      930      932       +2     
==========================================
+ Hits         4884     4891       +7     
  Misses        484      484
Impacted Files Coverage Δ
src/sidebar/components/hypothesis-app.js 91.01% <100%> (+0.76%) ⬆️

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 5a2e918...e85c48b. Read the comment docs.

@seanh
Copy link
Contributor

seanh commented Dec 7, 2017

@robertknight Can we just delete this banner from the product? It's stupid. Dan and Dawa previously agreed: https://hypothes-is.slack.com/archives/C07NXBDNW/p1510080494000386

@robertknight
Copy link
Member Author

OK, even better. I'll close this PR then.

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.

2 participants