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 #274

Merged
merged 8 commits into from
Oct 10, 2017
Merged

Conversation

wbobeirne
Copy link
Contributor

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

(function() {
var badBrowser = false;

var badBrowser = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicate?

@dternyak
Copy link
Contributor

dternyak commented Oct 10, 2017

Windows 7 with IE 8 doesn't properly show the old browser text, but other than that I had no issues 👍. It's likely not worth the effort to troubleshoot IE8, so I'm inclined to merge aggressively so as not to spend time debugging this.

screen shot 2017-10-10 at 2 50 00 pm

@james-prado james-prado self-requested a review October 10, 2017 22:18
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