[Fixes #1548] Watch for escape button being pressed

[kkirsche]

Fixes Issue #1548

First we define the jQuery object as $ using requirejs (line 6), we then use jQuery to bind the keyup event to the body (line 61). When the event occurs we use the escapeEvent function (lines 76–82) to verify that it was the escape key (keycode 27) that was pressed. If it was, we then use $scope.close() to close the opened "panel" and then unbind the event from the body to ensure we aren't using up memory / cpu for no reason.

[spalger]

We should probably be binding to the element here, not the body.

[w33ble]

.bind and .unbind have been deprecated as of jQuery 1.7 in favor of .on and .off. Also, element here is a jQuery-ified version of the config DOM node, so you should be able to use element.on

[spalger]

Thanks @kkirsche! I left a couple comments in the code.

[w33ble]

Overall, I'm not crazy about this implementation. It only works when an input inside the config directive is focused. This means it won't work as expected when using with anything but the save option since the other config options don't auto-focus an input. For example, on the dashboard:

1. Click save
2. Press ESC

This will close the config, since the save input is focused.

1. Click open
2. Press ESC

Nothing happens, since no input is focused here.

Binding to body would fix this, but that's going to cause issues when we want to add the same kind of escape to close logic to other directives (the timepicker comes to mind). We could bind to body and use the delegate syntax, but that still feels hacky and doesn't solve the problem of which dialog to close when a user presses ESC.

[kkirsche]

Thank you for the feedback.

I can understand the potential issues @w33ble when expanding this to other aspects. Since seeing your comment I have been trying to figure out how to identify what to close and sadly I have not thought of a solution to this yet.

Some ideas I have been trying to think through to see if there is a way they could address the issue include doing the binding with a directive that has $rootscope watch for the keypress and then $broadcast the namespaced event which the controller could catch. By namespacing the event with panel:escape we could close the right things where timepicker:escape could pick be fired when that is opened. I'm not convinced that's the cleanest solution though or that it would fully work as I'm thinking it. For that reason I've been trying to think of other options. If anyone has suggestions for what route might be best to address this I'll happily work on it or research any ideas that others have as I'm just a little stuck on how would be best to implement it.

Thank you for your time and understanding. I appreciate your patience.

[w33ble]

Yeah, I don't have a solid idea for how to handle this either. I had a similar idea though about using some "manager" higher up that would keep track of what collapsible menus had been opened and control what to do (if anything) when you hit escape.

As a basic example, if you open the timepicker, and then open the load dialog, pressing escape would close the last thing you opened (the load dialog). Pressing it again would close the next thing in the queue - last in first out. But, I'm not really sure that's right either... it makes sense if you do this quickly as part of an immediate flow, but if you open the time picker and then move around the application and hit ESC some time later, should it close the timepicker?

Maybe I'm just being too picky about it and hitting ESC when an input is not focused should just close any type of collapsible menu.

[rashidkpc]

I'm not sure how to feel about this either. I don't love the behavior of closing all of the open config directives on ESC, but I don't hate it either, and it seems better than ESC not working at all?

Closing them in a queue fashion works. I think ideally you'd want to close them in the order in which you last interacted with them, not in which they were opened. Not sure if thats possible to track cleanly.

[kkirsche]

I'm not sure how we would track interaction, depending on how we want to define interaction. If we define interaction as the opened panel that was most recently changed (with open / close qualifying for a "change" in this definition), it may be possible to use a function to update the queue order using ng-change / on creation of the panel.

[spalger]

Ideally, the escape handler we should try to find a config dropdown to close with the following lookup:

• parents of the currently focused element
• the most recently interacted with config
• the most recently opened config

The first config found by this lookup would then be closed.

To track the opened and interacted with configs, a service should be created that each config directive will communicate with.

"Interacted with" should probably be tracked using an event listener on the top level element that surrounds the config and all of it's panes, which listens for click and focus events.

[rashidkpc]

I'm going to go ahead and close this for now, feel free to update and resubmit.