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

Switch from setupCustomHover to hoverService #166500

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

rzhao271
Copy link
Contributor

@rzhao271 rzhao271 commented Nov 16, 2022

Ref #159088

By switching to hoverService, we get to customize the hover widget
more for the Settings editor indicators.

The PR also adds keydown handlers and tabIndexes for the indicators so that one can focus onto an indicator and then press a key such as Enter to show the hover.

@rzhao271 rzhao271 self-assigned this Nov 16, 2022
Ref #159088
By switching to hoverService, we get to customize the hover widget
more for the Settings editor indicators.
@rzhao271 rzhao271 force-pushed the rzhao271/sei-remove-setupcustomhover branch from 10fe16b to dd01fd8 Compare November 16, 2022 21:44
@rzhao271 rzhao271 marked this pull request as ready for review November 16, 2022 22:34
@rzhao271 rzhao271 added this to the November 2022 milestone Nov 16, 2022
@rzhao271
Copy link
Contributor Author

After this PR, there's still hover focus to fix, as well as getting the left/right keys to work with the indicators.

Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

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

Seems fine, but if you wanted a really in-depth review, let me know

@rzhao271 rzhao271 merged commit c3f40b9 into main Nov 17, 2022
@rzhao271 rzhao271 deleted the rzhao271/sei-remove-setupcustomhover branch November 17, 2022 18:25
@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2023
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.

2 participants