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

Add warning for users visiting Grants page #5988

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

sebastiantf
Copy link
Contributor

Description

Adds a warning for users about using Brave Wallet and MetaMask while visiting https://gitcoin.co/grants

grantswarning

Refers/Fixes

Fixes: #5865

Testing

No issues on eslint and stylelint.

@codecov
Copy link

codecov bot commented Feb 14, 2020

Codecov Report

Merging #5988 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5988      +/-   ##
==========================================
+ Coverage   28.86%   28.89%   +0.02%     
==========================================
  Files         270      270              
  Lines       23742    23742              
  Branches     3454     3454              
==========================================
+ Hits         6854     6860       +6     
+ Misses      16615    16609       -6     
  Partials      273      273
Impacted Files Coverage Δ
app/dashboard/embed.py 31.6% <0%> (+3.44%) ⬆️

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 d99749b...2617a23. Read the comment docs.

@thelostone-mc
Copy link
Member

@sebastiantf

This would probably be most effective on the /fund page when users are about to contribute to grant and the grant creation page (/grants/new)

What do you think on restricting the alert to just those pages?
Also If we can figure out if user is using brave and only show it then -> it's super effective

a quick google led me to this
https://stackoverflow.com/questions/36523448/how-do-i-tell-if-a-user-is-using-brave-as-their-browser

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Feb 14, 2020

@thelostone-mc Showing the warning on /fund and /grants/new seems like a good idea. That can be done.

But as I mentioned on the issue page, Brave browser doesn't seem to provide a Brave-specific User Agent which makes detecting Brave browser difficult. See #1052

And since Brave Wallet is a fork of MetaMask, the web3.currentProvider.isMetamask returns true.

So it seems displaying the warning only for Brave users by detecting Brave Browser or Brave Wallet is hard.

The answer in the stackoverflow question also says:

The "Brave" in the user agent was removed in the 0.9 version.

From the changelog:

Removed Brave from the User Agent HTTP header to reduce fingerprinting.

@thelostone-mc
Copy link
Member

@sebastiantf let's do it that way !
If I see a way to detect it -> i'll throw that in later

Danke :D

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Feb 14, 2020

@thelostone-mc Shall I change the warning from /grants to /fund and /grants/new?

@sebastiantf
Copy link
Contributor Author

@thelostone-mc Please check my new commits and review this PR. I have changed the warning from /grants to /fund and /grants/new

@thelostone-mc
Copy link
Member

@sebastiantf Looks good ^_^

@thelostone-mc thelostone-mc merged commit 69cb771 into gitcoinco:master Feb 17, 2020
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.

Add warning for Brave users
2 participants