Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Close open navigation items with escape. #253

Merged
merged 1 commit into from
Jun 10, 2015

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Jun 9, 2015

[Resolves #872]
[Resolves #934]

@@ -3,6 +3,7 @@
/* global require, window, document */

var $ = require('jquery');
var keyboard = require('keyboardjs');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need a whole library if we're just going to use a mapping of keys to key codes? Are we going to be using this library elsewhere for more in depth functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need it, but copying and pasting key codes from the internet is extra work and error-prone. We're also using the library in the typeahead module, but not in depth--really, all it does is key mapping lookups and a more concise event listener. It's about 10k minified. This would be easy to revert if you want--I kind of like it, but no strong preferences.

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I could see doing more with keyboard shortcuts down the road but
don't feel too strongly about it or whether we should include a library.

On Tue, Jun 9, 2015 at 4:32 PM, Joshua Carp notifications@github.com
wrote:

In static/js/init.js
#253 (comment):

@@ -3,6 +3,7 @@
/* global require, window, document */

var $ = require('jquery');
+var keyboard = require('keyboardjs');

We don't need it, but copying and pasting key codes from the internet is
extra work and error-prone. We're also using the library in the typeahead
module, but not in depth--really, all it does is key mapping lookups and a
more concise event listener. It's about 10k minified. This would be easy to
revert if you want--I kind of like it, but no strong preferences.


Reply to this email directly or view it on GitHub
https://github.com/18F/openFEC-web-app/pull/253/files#r32076153.

Noah Manger
18F http://18f.gsa.gov | Work: 415-696-6146

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have trouble with our assets being too heavy, then I'm 👍 to including a library for convenience.

msecret pushed a commit that referenced this pull request Jun 10, 2015
Close open navigation items with escape.
@msecret msecret merged commit 4725ccd into fecgov:develop Jun 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants