Skip to content

added modal to check if chrome browser #531

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

added modal to check if chrome browser #531

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e104fb2
added modal to check if chrome browser
leesan11 Jun 5, 2018
1513a4f
changes to modal, added cookie feature
leesan11 Jun 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions create.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,18 @@ <h4 class="modal-body text-danger" :style="{display: !getID() ? 'block' : 'none'
</div>
</div>
</div>

<div id="chromeModal" class="modal fade" tabindex="-1" role="dialog">
<div class="modal-dialog" role="document">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
<h4 class="modal-title"><span class="glyphicon glyphicon-share-alt"></span> Chrome Browser Not Detected!</h4>
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the share glyhicon <span class="glyphicon glyphicon-share-alt"></span>. It's ok to copy and paste code, but do be sure do remove the code that's no longer relevant

Copy link
Owner

Choose a reason for hiding this comment

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

I'd probably rename the title to "WoofJS is Optimized for Chrome"

</div>
<h4 class="modal-body text-danger" :style="{display: !getID() ? 'block' : 'none'}">The Chrome Browser is recommended. You may experience technical difficulties with other browsers.</h4>
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this here? {display: !getID() ? 'block' : 'none'}

Copy link
Owner

Choose a reason for hiding this comment

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

You could add a link to download chrome: https://www.google.com/chrome/

Copy link
Owner

Choose a reason for hiding this comment

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

I'd re-word this: "WoofJS.com works best with the Chrome web browser. We're a small team, so we can't test new features on all browsers. If you are having difficulties with WoofJS.com, consider downloading Chrome here."

</div>
</div>
</div>

<div id="docsbar" v-show="docsbar" @click="docsbar= !docsbar" :style="{display: 'flex'}">
<span class="rotate" style="font-size: 1.65em">Documentation</span>
Expand Down Expand Up @@ -1690,6 +1702,16 @@ <h4 class="modal-body text-danger" :style="{display: !getID() ? 'block' : 'none'
// helper function to prevent runCode from running too often
var debouncedRunCode = debounce(runCode, 1000, false)

function checkChrome() {

Copy link
Owner

Choose a reason for hiding this comment

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

remove empty line here

var str = window.navigator.userAgent;
Copy link
Owner

Choose a reason for hiding this comment

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

I'd probably make this a const declaration and change the variable name to userAgent

if (!(str.indexOf("Edge") == -1 && str.indexOf("OPR") == -1 && str.indexOf("Firefox") == -1 && str.indexOf("Trident") == -1)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Usually when you have a very specific expression like this that you got from the internet (I'm guessing), you want to include the URL where you got it from for context as a comment in your code.

From my 5 min of research, this seems like the most reliable answer.

$("#chromeModal").modal("show");
Copy link
Owner

Choose a reason for hiding this comment

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

It doesn't really matter, but I'd consider making checkChrome() return a boolean, and then show the model if it's true. If you wanted to do that, you could rename the method to isChromeBrowser so that it reads better as if(!isChromeBrowser()) { $("#chromeModal").modal("show"); }

}

Copy link
Owner

Choose a reason for hiding this comment

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

remove empty line

};
checkChrome();

</script>
<script>
(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
Expand Down