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

Move generic hover widget to base ui and extend in ContentHoverWidget #97496

Closed
Tyriar opened this issue May 11, 2020 · 5 comments · Fixed by #100285
Closed

Move generic hover widget to base ui and extend in ContentHoverWidget #97496

Tyriar opened this issue May 11, 2020 · 5 comments · Fixed by #100285
Assignees
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded workbench-hover Hover issues in the workbench
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented May 11, 2020

No description provided.

@Tyriar Tyriar added feature-request Request for new features or functionality workbench-hover Hover issues in the workbench labels May 11, 2020
@Tyriar Tyriar added this to the May 2020 milestone May 11, 2020
@Tyriar Tyriar self-assigned this May 11, 2020
@Tyriar
Copy link
Member Author

Tyriar commented May 11, 2020

We could also expose the widget in a HoverService for ease of use in the workbench, this would handle:

  • Ensuring only 1 is shown at a time
  • Timeout scheduling, resetting if the mouse moves
  • Easy breaking out of the container

See dialog and context menu service for similar services.

@rebornix
Copy link
Member

Timeout scheduling, resetting if the mouse moves

FYI we may also want to reset the hover when the target element's position changes in the workbench. We had an issue in Notebook that when the notebook scrolls, the hover doesn't go away as its relative position to the Monaco Editor container doesn't change. Now with the hover being promoted to workbench/service level, we can fix this issue by having position monitoring on the hover target. (but not sure about the perf penalty)

@Tyriar
Copy link
Member Author

Tyriar commented May 18, 2020

@rebornix the notebook would still use the editor's version and its lifecycle management though. For the workbench-level hover the plan is to ensure only a single hover shows at a time.

@Tyriar
Copy link
Member Author

Tyriar commented Jun 29, 2020

To verifier: test the hover and its overflow behavior by:

  • Writing links in the terminal
  • Run the terminal-sample extension and Update Environment/Clear Environment commands to show the environment variable indicator. More entries can be added here to make the hover bigger and test bounds overflow behavior

@Tyriar Tyriar added the verification-needed Verification of issue is requested label Jun 29, 2020
@connor4312 connor4312 added the verified Verification succeeded label Jul 1, 2020
@connor4312
Copy link
Member

connor4312 commented Jul 1, 2020

Well, since you asked for overflow behavior 😄 It looks like if you set too many variables to extends over the height of the vscode window. Maybe we need max-height: 90vh or something on it.

Seems to work well with sane values though

@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded workbench-hover Hover issues in the workbench
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@rebornix @Tyriar @connor4312 and others