Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

JS Keycodes #1574

Closed
wants to merge 9 commits into from
Closed

JS Keycodes #1574

wants to merge 9 commits into from

Conversation

andrey-p
Copy link
Contributor

Thought a JS keycodes IA is something that's needed. There's a goodie that does a similar thing but it hasn't been released yet. (Further discussion between me and @mintsoft here.)

What does your Instant Answer do?

Displays Javascript keycodes.

What problem does your Instant Answer solve (Why is it better than organic links)?

It saves a few clicks for a very common (for front-end devs) search topic.

What is the data source for your Instant Answer? (Provide a link if possible)

The existing Goodie and this page, which is the first result for "js keycodes" (and I've been using it for years).

Are there any other alternative (better) data sources?

There's the MDN page on this too.

What are some example queries that trigger this Instant Answer?

"keycode tab", "javascript keycode j", "javascript keycodes"

Which communities will this Instant Answer be especially useful for? (gamers, book lovers, etc)

Javascript developers.

Is this Instant Answer connected to a DuckDuckHack Instant Answer idea?

Nope.

Which existing Instant Answers will this one supersede/overlap with?

The JsKeycodes goodie

Are you having any problems? Do you need our help with anything?

Turns out there's a few weird cross-browser / cross-platform (possibly even cross-keyboard and cross-locale?) differences - they're the ones highlighted in the massive MDN table linked above. I decided to go for the data that was already there, but I was wondering whether it'll be a good idea to add a disclaimer mentioning there could be some differences.

What are the terms of use for the API? Will DuckDuckGo need specific authorization (e.g. an API key)? Are there any costs associated with API usage?

None.

What does the Instant Answer look like? (Provide a screenshot for new or updated Instant Answers)

2015-02-27-150657_1366x768_scrot
2015-02-27-151102_1366x768_scrot
2015-02-27-151114_1366x768_scrot

[X] Added metadata and attribution information
[X] Wrote test file and added to t/ directory
[X] Verified that instant answer adheres to design guidelines (https://duck.co/duckduckhack/design_styleguide)
[X] Verified that instant answer adheres to code styleguide (https://duck.co/duckduckhack/code_styleguide)
[X] Tested cross-browser compatibility (see note above though)

- Linux
      [X] Chromium
      [X] Firefox   

- Windows XP
      [X] IE 8

IA Page: https://duck.co/ia/view/js_key_codes

- added check in case keycode isn't in our list
- added source
- added some comments
- removed Spice.failed - that way we can display the tester
  if the search is something like "javascript keycodes list"
# display only if query contains "javascript" or "js"
# or the remainder is a valid key
# to avoid conflict with any other uses of the word "keycode"
return unless $_ =~ s/js|javascript//i or exists $keys{$_};
Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid calling this Spice on any search for keycode can the js,javascript triggers be added to triggers instead of the guard.

This will also allow you to return the keycode and avoid using .replace on L58 in js_keycodes.js by grabbing the returned result using source.match - the docs you wrote!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid calling this Spice on any search for keycode can the js,javascript triggers be added to triggers instead of the guard.

@MrChrisW My approach was to trigger on searches like keycode [valid keyname] (but not searches like keycode pro) - do you think it'll be more useful to allow only searches with js and javascript in the query?

This will also allow you to return the keycode and avoid using .replace on L58 in js_keycodes.js by grabbing the returned result using source.match - the docs you wrote!

Yeah, I was pretty excited about using that as well but I couldn't get it to work for this spice - I think having call_type => 'self' means there's no separate JSONP call to the server.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@andrey-p Bugger! That's right, unfortunately there's nothing passed to the front end when using 'self'

Regarding the triggering, I think it's probably a good idea to only trigger when js or javascript is contained in the query (feel free to disagree! 😄 ) my reasoning behind this is because these keycodes are Javascript specific, they're not Ascii decimal keycodes for example.

You can also improve the triggering by assigning a list of triggers that you've generated with map to triggers startend => this will avoid calling the handle for useless queries (keycode pro) and thus improve overall performance.

You can find a good example of this in the RandPOS IA!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding the triggering, I think it's probably a good idea to only trigger when js or javascript is contained in the query (feel free to disagree! 😄 ) my reasoning behind this is because these keycodes are Javascript specific, they're not Ascii decimal keycodes for example.

I concur with this; it's better to be more precise and expand later than too broad and give users irrelevant information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my reasoning behind this is because these keycodes are Javascript specific, they're not Ascii decimal keycodes for example.

Ah, that's an excellent point. I'll change it so that it always checks for js or javascript in the query.

You can also improve the triggering by assigning a list of triggers that you've generated with map to triggers startend => this will avoid calling the handle for useless queries (keycode pro) and thus improve overall performance.

That's a good trick to keep in mind, thanks. Might not end up using it as I might remove the list of key names from the backend - will have a play around with it and see.

- disabled trigger on searches like "keycode tab" as this may conflict
  with ex. ASCII keycodes
- added triggers for "key code"
@moollaza
Copy link
Member

@andrey-p this is looking pretty good -- I noticed a few keys didn't register around my numpad, but that may be my keyboard?

However the - and = keys beside my backspace didn't seem to register either?

@abeyang any design thoughts? My biggest concern is the mobile layout because the table is large and wide

<a class="btn btn--primary show-keycodes" href="#">Show all codes</a>

{{! hidden when the page first renders }}
<table class="all-keycodes">
Copy link
Member

Choose a reason for hiding this comment

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

Might be best to add the class "is-hidden" here so that it doesn't need to wait for the Spice's CSS to load in order to hide it, or you could use inline css but the former is better 😃

- hiding the table initially with is-hidden
- moving the initial interaction setup to Spice.onShow
@andrey-p
Copy link
Contributor Author

@moollaza Added changes to do with hiding the table with is-hidden and moving stuff to onShow.

I noticed a few keys didn't register around my numpad, but that may be my keyboard?

However the - and = keys beside my backspace didn't seem to register either?

Yeah, that's the thing I'm not 100% what to do about... Turns out there's some minor but very annoying differences between the way different keyboards, browsers and OSes handle keycodes. For instance this page on MDN lists about 3 different keys that can have a keycode of 186!

I was thinking of adding a link to the MDN article and a disclaimer that some key combinations are ambiguous, maybe marking the offending entries in the table in some way. What do you think?

@moollaza
Copy link
Member

@andrey-p I think the More at link should probably link to MDN -- a disclaimer would be good too!

@jagtalon
Copy link
Member

jagtalon commented Apr 8, 2015

@moollaza looks like it's ready for review!


# Handle statement
handle remainder_lc => sub {
return call;
Copy link
Member

Choose a reason for hiding this comment

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

We should check here that if there's a remainder it's relevant. I.e. just a single word -- if we could make sure it's exactly an expected keycode that would be awesome. We could build a hash (perhaps in a separate text file in /share/) that contains all the names of the keys and if the remainder is one of those, or non-existant this AI can return, otherwise it should ignore the query.

We typically use YAML and load the strings into a hash for a fast lookup. This is a good example: https://github.com/duckduckgo/zeroclickinfo-spice/pull/1737/files

@gautamkrishnar
Copy link
Collaborator

👍

@jagtalon
Copy link
Member

jagtalon commented Sep 3, 2015

Talked to @andrey-p and @moollaza and we agreed that this would be better as a cheat sheet (albeit a modified cheat sheet that has the same features as this one). Closing this in favor of that. :neckbeard:

@jagtalon jagtalon closed this Sep 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants