search

georgemandis / konami-js

Adding the Konami Code easter egg to your projects since 2009! Compatible with gestures on smartphones and tablets as well. Compatible with all front-end frameworks and vanilla JavaScript
http://konamijs.mand.is/
MIT License
959 stars 122 forks source link

Fix for chrome not working #20

Closed sqrtroot closed 8 years ago

sqrtroot commented 8 years ago

Chrome input didn't work- fixed

georgemandis commented 8 years ago

What kind of error were you getting on Chrome? I've never heard or experienced this issue before, but I am seeing how we could rewrite that line now.

sqrtroot commented 8 years ago

@georgemandis the plugin didn't work on chrome for me (chrome on pc and chrome on mobile), it just didn't register my key events. This fixed that.

sukima commented 8 years ago

Per the MDN keyCode should not be used:

Deprecated This feature has been removed from the Web standards. Though some browsers may still support it, it is in the process of being dropped. Do not use it in old or new projects. Pages or Web apps using it may break at any time.

Also:

The value of keypress event is different between browsers. IE and Google Chrome set the KeyboardEvent.charCode value.

georgemandis commented 8 years ago

This whole issue is a lot more interesting than I thought. Per the MDN it looks like all the following shouldn't be used anymore:

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/charCode https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/which

It appears the properties it would want us to use are either:

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key

But those don't seem to have a lot of browser support. They require rewriting other parts of the code as well because it'd be returning a string instead of a number ("ArrowUp" vs 38).

Frustratingly, it also appears that Safari/Webkit doesn't support either of these new properties — code will work on Chrome and Firefox and key only on Firefox and IE9+.

Future-proofing the script to account for those properties should probably happen at some point, but I'd want to wait until one or the other gains more cross-browser traction.

As for your specific problem, a couple things:

  1. This is still the first I've heard of this and can't recreate the problem. Can you open up this page — http://snaptortoise.com.s3.amazonaws.com/konami-test.html — and confirm for me that it's not working? It works for me on both Mac and PC.
  2. If we do need to update things with a fix, the double-ternary operator approach in your script doesn't strike me as necessary:

konami.input += e.charCode ? e.charCode : e.keyCode ? e.keyCode : 0;

Technically we shouldn't be using either per MDN, I guess, but I'm assuming for your code change that keyCode wasn't working for you. It seems to me it should be more like

konami.input += e.keyCode ? e.keyCode : e.charCode;

I don't think there should be a scenario where we're returning zero.

If my earlier link is working correctly for you then I think it must be something else in the code on the page you were working on that created the problem. In that case I would not merge this pull request, but I would be happy to continue discussing what an update to "future-proof" this might look like.

sqrtroot commented 8 years ago

hmm on the page you did send me it works, on my live page it didn't work strange. I think I'll keep it as a fix for my own page. and close this one. Future proofing might be a good idea IF the api's stabilize

georgemandis commented 8 years ago

Sounds like there's something else going on with your page that may have been breaking things. If you wanted to link to it I'd be happy to take a look and help. Curious why it would break or why your fix would fix it!

Agree on the future-proofing. I'll open an issue and reference this conversation and maybe someone, somewhere, someday will tend to it... :)