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

[Feature] Add a mechanism to persist the content hover size #185743

Closed
aiday-mar opened this issue Jun 21, 2023 · 3 comments · Fixed by #187557
Closed

[Feature] Add a mechanism to persist the content hover size #185743

aiday-mar opened this issue Jun 21, 2023 · 3 comments · Fixed by #187557
Assignees
Labels
editor-hover Editor mouse hover insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@aiday-mar
Copy link
Contributor

aiday-mar commented Jun 21, 2023

Add the possibility to persist the content hover sizes

@aiday-mar aiday-mar added editor-hover Editor mouse hover feature-request Request for new features or functionality labels Jun 21, 2023
@aiday-mar aiday-mar changed the title Add a mechanism to persist the content hover size [Feature] Add a mechanism to persist the content hover size Jun 21, 2023
@aiday-mar aiday-mar self-assigned this Jun 21, 2023
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Jul 11, 2023
@vscodenpa vscodenpa added this to the July 2023 milestone Jul 11, 2023
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jul 12, 2023
@aiday-mar aiday-mar added the verification-needed Verification of issue is requested label Jul 25, 2023
@aiday-mar
Copy link
Contributor Author

aiday-mar commented Jul 25, 2023

To test, please read the following.

Currently the implementation of the persistance mechanism of the content hover works as follows:


The content hover has a default maximum size which is defined as follows:

const defaultMaxHeight = Math.max(this._editor.getLayoutInfo().height / 4, 250);
const defaultMaxWidth = Math.max(this._editor.getLayoutInfo().width * 0.66, 500);

Whenever the content hover is resized, the last hover dimensions are stored. Next time the content hover is shown, the maximum dimensions are updated as follows:

const currentMaxHeight = Math.max(this._editor.getLayoutInfo().height / 4, 250, lastHeight);
const currentMaxWidth = Math.max(this._editor.getLayoutInfo().width * 0.66, 500, lastWidth);

The content hover is rendered naturally within these constraints. If the lastHeight and lastWidth are smaller than the default max dimensions, then the current max dimensions are unchanged. Therefore the next rendering of the content hover will be the same as the initial rendering.


Test that the content hover works according to these constraints. There is some discussion going on about how to better implement a persistance mechanism. We could also persist the size for example on a per-token basis in the editor. Let me know what you think is best!

@hediet
Copy link
Member

hediet commented Jul 25, 2023

Do you have an example of how it should work? Regardless of how I resize it, it is not remembered.

@hediet hediet added the verification-steps-needed Steps to verify are needed for verification label Jul 25, 2023
@aiday-mar
Copy link
Contributor Author

aiday-mar commented Jul 25, 2023

You may see an example of how it works here. The content hover is resized to a bigger size in the gif. The current max width is increased to a higher value and at the next rendering of the content hover, the widget takes on a bigger size. To see this behavior you should choose a content hover where the content does not fit in the visible widget, where there is a horizontal scrollbar.

Screen.Recording.2023-07-25.at.14.36.16.mov

I opened an issue for persisting the content hover on a per-token basis here: #187904 as an alternative idea.

@aiday-mar aiday-mar removed the verification-steps-needed Steps to verify are needed for verification label Jul 25, 2023
@hediet hediet added the verified Verification succeeded label Jul 25, 2023
@aiday-mar aiday-mar removed the feature-request Request for new features or functionality label Jul 25, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-hover Editor mouse hover insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants