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

fix(Tooltip): adds esc listener #2248

Merged
merged 9 commits into from
Apr 24, 2019

Conversation

paschalidi
Copy link
Contributor

@paschalidi paschalidi commented Apr 19, 2019

Hello 馃憢 !

Closes https://github.com/IBM/carbon-components-react/issues/1764

Adds event listener so that Esc key closes Tooltip when state is open.

Changelog

New

Adds event listener so that Esc key closes Tooltip when state is open.

@netlify
Copy link

netlify bot commented Apr 19, 2019

Deploy preview for carbon-components-react ready!

Built with commit 2102a0a

https://deploy-preview-2248--carbon-components-react.netlify.com

@@ -266,6 +266,8 @@ class Tooltip extends Component {
requestAnimationFrame(() => {
this.getTriggerPosition();
});

document.addEventListener('keydown', this.handleEscKeyPress, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clean-up this event listener on un-mount.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the quick review! Keep it up!

Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

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

This is cool. Thanks! When @asudoh's fixes are in looks good to me. 馃槑

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 馃憤 - Thanks @paschalidi!

@paschalidi
Copy link
Contributor Author

@asudoh is something holding us back here?

@asudoh
Copy link
Contributor

asudoh commented Apr 24, 2019

@paschalidi Sorry for being late!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants