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

Align hover with status bar entry #125090

Closed
joaomoreno opened this issue Jun 1, 2021 · 7 comments
Closed

Align hover with status bar entry #125090

joaomoreno opened this issue Jun 1, 2021 · 7 comments
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders
Milestone

Comments

@joaomoreno
Copy link
Member

joaomoreno commented Jun 1, 2021

Testing #125005

Hover should be left-aligned with the status bar area. Pulling the mouse upwards will make the hover disappear because the hover isn't right above the mouse.

recording (4)

@aeschli aeschli added this to the May 2021 milestone Jun 2, 2021
@aeschli
Copy link
Contributor

aeschli commented Jun 2, 2021

@joaomoreno It's probably the wrong screenshot...

@joaomoreno
Copy link
Member Author

Fixed! 😆

@aeschli aeschli closed this as completed in 1d47121 Jun 2, 2021
Tyriar added a commit that referenced this issue Jun 2, 2021
This reverts commit 1d47121.

This broke the build, hover delegate is used implicitly elsewhere
@Tyriar
Copy link
Member

Tyriar commented Jun 2, 2021

I reverted the fix here, IHoverDelegate is used implicitly elsewhere. I'm not sure why this is needed also, can you do a PR for this? I'd prefer not to have another mandatory property if it can be avoided to keep things simple.

@Tyriar Tyriar reopened this Jun 2, 2021
@Tyriar
Copy link
Member

Tyriar commented Jun 2, 2021

60a3621

@aeschli aeschli closed this as completed in 20be2c8 Jun 2, 2021
@aeschli
Copy link
Contributor

aeschli commented Jun 2, 2021

@Tyriar Sorry for the breakage, yarn watch didn't catch the implicit use in terminal.
I discussed the option prior with @alexr00. Sorry, didn't know you also had a stake in it.

I made the placement optional: ce8d2e8

@Tyriar
Copy link
Member

Tyriar commented Jun 2, 2021

@aeschli does this belongs more in IHoverDelegateOptions as it's defining the positioning of the hover like HoverPosition?

@aeschli
Copy link
Contributor

aeschli commented Jun 3, 2021

The IHoverDelegateOptions is an input for the hover delegate. (Used when the hover delegate is asked to show the hover hoverDelegate.showHover)

What could be done that each hover delegate does the positioning itself in hoverDelegate.showHover. But that would duplicate the positioning code (hoverOptions.target)x = mouseX + 10; at many places. Right now hoverDelegate.showHover is just a forward to the hover service.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Development

No branches or pull requests

4 participants
@joaomoreno @Tyriar @aeschli and others