Uses new WindowEvent component for Flyout "close on ESC"

[jasonrhodes]

The keyDown event listener for the Flyout component was attached to the Flyout node itself, so if you clicked anywhere outside of the Flyout and pressed "ESC", the Flyout would remain open. This PR does the following:

• Defines a new EuiWindowEvent component to safely handle the adding and necessary unmount-removal of window events
• Adds a few tests for the new component
• Uses the new component to define a window listener for closing the flyout on ESC press

I built the code locally and ran in local Kibana and it works as described here. The local build fails on the webpack bundle step, which I'm looking more into now.

Update: local webpack error was a circular dependency because I was importing directly from components instead of from components/window_event ... all fixed now :)

[sorenlouv]

Do we still need the event listener on the flyout?

eui/src/components/flyout/flyout.js

Line 75 in 6c6d790

onKeyDown={this.onKeyDown}

Won't onKeyDown be invoked twice now? Both by the listener on the div and EuiWindowEvent?

[jasonrhodes]

@sqren good catch -- I'd removed that and then accidentally killed my changes in that file with git checkout -- . and forgot to re-remove it. Thanks!

[jasonrhodes]

Note: this PR addresses a TODO item in elastic/kibana#21300

[sorenlouv]

LGTM! You also need a 👍 from someone on the EUI before merging.

[cchaos]

Thanks for jumping in here to add this.

A couple of things:

1. Can you actually move this to the services folder instead of components as it doesn't actually render any dom elements but is just a helper?
2. Can you also add a documentation page under the Utilities category with some more explanation of what it does and why one would need to use it.
3. Don't forget to add an entry to the Changelog.

[cchaos]

I don't think you need the default as here as it's in the component file.

[jasonrhodes]

This is how you "proxy export" a default component from another file, I think. Or I could make the export in window_event be named and drop default as here.

[cchaos]

Yeah I think we just name everything so we ensure there are no conflicts or that we can add to them down the line without breaking changes.

[jasonrhodes]

Hey @cchaos, I just pushed some example/docs and changelog additions, but please let me know if I've done anything funky in there. I'll add a bit more explanation to what I've got in the docs stuff, and I'll move the component into services in a bit. Thanks for the feedback!

[cchaos]

This is a copied heading, you can just append your bullets after the EuiSuperSelect one.

[jasonrhodes]

Addressed here: 73f0ed0

[cchaos]

Can you also describe how?

[jasonrhodes]

Sure!

[jasonrhodes]

Addressed: e4d90d9

[cchaos]

Also add a props key here to populate a props tab with the props listed in a table like so:
props: { EuiWindowEvent }

[jasonrhodes]

@cchaos I can't find the comment now due to GitHub magic, but re: your request for explaining "how" to use the component, that's addressed here: e4d90d9

[jasonrhodes]

New component moved to services directory here: a348dbb

[cchaos]

Here's an issue I saw using the example:

Is there any way to add scope or prevent something like this?

[jasonrhodes]

@cchaos re: scope, I think the answer is that my example is a bad one (or at least bad for that particular problem). Normally you wouldn't be listening for such a common key-press. If you were instead listening for "esc", for example, and there was a typeahead that was directly also listening on "esc" to clear results, the typeahead would do e.stopPropagation() which would prevent the window from hearing the event.

In other words, window events will always fire unless a more targeted event stops the event propagation earlier, so they need to be less generic than "any backspace key press".

I can change that to "escape" in the example if you think that would help? And/or make a note to this effect in the example docs?

[cchaos]

Yeah, I think I would err on the side of creating an example that is the proper use of the service. We see a lot of just copy and paste style usages and want to make sure that doing so doesn't introduce issues. But I also think adding another paragraph or even a callout, to describe this issue would be good as well.

[cchaos]

These new examples are fantastic. I think it's a lot clearer when and why to use this helper. Thanks so much for adding it all.

Just had one more adjustment to add, but LGTM!

[cchaos]

Add props: { EuiWindowEvent }