Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tooltip should fade on keybaord event 17431 #17651

Merged
merged 5 commits into from Jul 10, 2018

Conversation

Projects
None yet
4 participants
@cacheflow
Copy link
Contributor

cacheflow commented Jul 9, 2018

Description of the Change

Fix #17431 by adding an event listener that listens for keydown events and hides a tooltip when a user types.

Alternate Designs

  1. Initially I had attached the event listener in the tooltip file instead of the tooltip manager, but I placed in the latter because that's where the hiding tooltips on resized lived as well.

Why Should This Be In Core?

Because it fixes a problem in core and most default editors have this feature.

Benefits

It fixes an issue where users may want to see the workspace they're typing in, but a tooltip is hiding it.

Possible Drawbacks

It adds an additional event listener for each tooltip.

Verification Process

I modified the existing tooltip-manager spec to include a new test for this feature, ran the test suite, and also manually tested it by reproducing the steps listed in #17431.

Applicable Issues

#17431

cacheflow added some commits Jul 6, 2018

Hides a tooltip when a user types in the editor.
-This addes a new keydown event that hides a tooltip when keyboard events occur.
-This closes #17431.

@cacheflow cacheflow force-pushed the cacheflow:tooltip-should-fade-on-keybaord-event-17431 branch from abc173e to 4d48ea8 Jul 9, 2018

@50Wliu

50Wliu approved these changes Jul 10, 2018

Copy link
Member

50Wliu left a comment

Makes sense to me!

@thomasjo
Copy link
Member

thomasjo left a comment

LGTM

@lee-dohm lee-dohm merged commit 5a6b3c6 into atom:master Jul 10, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Jul 10, 2018

Thanks very much for the awesome contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.