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

No Javascript / Old Browser Blockers #280

Merged
merged 10 commits into from
Oct 10, 2017
Merged

No Javascript / Old Browser Blockers #280

merged 10 commits into from
Oct 10, 2017

Conversation

dternyak
Copy link
Contributor

Originally MyEtherWallet/MyEtherWallet#274

What This Does

Closes out #79:

  • Adds a page blocker for if your browser is missing any of the following features:
    • const / let
    • Local storage
    • Object.assign
    • Flexbox support (Otherwise the site will look broken)
  • Adds a page blocker if you have Javascript disabled via a noscript tag

The copy for either of these is totally up or editing, I just threw something together. Please advise @tayvano.

How to Test

  1. Run npm run build
  2. Use http-server or something to serve up the dist/ folder
  3. Use out browserstack credentials (Found in slack) and load the page in an old browser to see the bad browser message
  4. Load the page in Chrome, and disable javascript to see the no js message like so:

screen shot 2017-10-08 at 12 10 06 pm

Janky Implementations

Because a few of the third party modules make it into our vendor bundle with a const here and a let there, I decided that to keep this as fail-proof as possible, I would do the bad browser check outside of the main code bundle. So you'll find it, along with all of the markup for these, at the bottom of index.html. This wasn't ideal, but after having the common or vendor bundles fail for a myriad of reasons, felt like a real gordian knot chop.

Also, because they're not served up via React, I couldn't make a nice common component for these kinds of page blockers. So instead I made a sass mixin, @mixin cover-message which applies the styles, but allows for customization and overrides.

Requires #273 to work due to needing to be tested with the npm run build version of this.

Screenshots

screen shot 2017-10-08 at 11 58 51 am

screen shot 2017-10-08 at 12 01 05 pm

@james-prado james-prado self-requested a review October 10, 2017 22:56
@dternyak
Copy link
Contributor Author

#274 wasn't re-targeted to develop before merging, and github won't let you re-target after merging (as expected), so I'm forced to re-open against develop.

@dternyak dternyak merged commit 2e472e5 into develop Oct 10, 2017
@dternyak dternyak deleted the browser-messages branch October 10, 2017 23:09
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.

3 participants