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

Debug Hover Widget - Enable the widget resizing for long items whose overflow is hidden #52511

Closed
liguori opened this Issue Jun 20, 2018 · 15 comments

Comments

@liguori

liguori commented Jun 20, 2018

It would be very useful to implement the resizing for the Debug Hover Widget. Sometime happen that during a debug session you want to check the content of a variable of a complex object. If this content overflows the default widget's width (324px), It is hidden.


Actually, the only way for check this content is to add the code expression into the "watch panel" or evaluate it trought the "Debug console". An alternative can be waiting for the tooltip that appear on the mouse hover at the specific node of the three.

vscodesample

@weinand weinand added the debug label Jun 20, 2018

@weinand weinand assigned isidorn and unassigned weinand Jun 20, 2018

@liguori

This comment has been minimized.

Show comment
Hide comment
@liguori

liguori Jun 20, 2018

Hi @weinand and @isidorn! I Have developed a very 'raw' implementation for the horizontal reisizing. Should i submit a PR?

liguori commented Jun 20, 2018

Hi @weinand and @isidorn! I Have developed a very 'raw' implementation for the horizontal reisizing. Should i submit a PR?

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Jun 21, 2018

Contributor

Our solution for this is setting
"workbench.tree.horizontalScrolling": true
Then you have a horizontal scroll and can see the whole value.
Thanks for offering a PR, but I prefer a current solution for now

Contributor

isidorn commented Jun 21, 2018

Our solution for this is setting
"workbench.tree.horizontalScrolling": true
Then you have a horizontal scroll and can see the whole value.
Thanks for offering a PR, but I prefer a current solution for now

@isidorn isidorn closed this Jun 21, 2018

@liguori

This comment has been minimized.

Show comment
Hide comment
@liguori

liguori Jun 21, 2018

Hi @isidorn
Your solution could be a good workaround. I opened this thread as "feature request" since the debug and inspection experience should be improved for more complex scenarios.

I checked the component source code files (debugHover.css & debugHover.ts) and i noticed that the widget sizes are hard coded as:
width= 324px;

height= item number (max 18) * 18px = max 324px; (

const height = Math.min(visibleElementsCount, MAX_ELEMENTS_SHOWN) * 18;
)

Trough 'git blame' i noticed that you are the author and for sure you should know better than everyone the ground up of this component.

I tried to "play" directly with the Developer Tools and some nice bheaviour could be reached without modify the JS/TS code for a basic horizontal resizing:

.monaco-editor .debug-hover-widget .complex-value {
	width:100%;
	min-width:324px;
	
}

.debug-hover-widget .monaco-scrollable-element {
	resize: both;
	padding-bottom: 15px;
}

I am new to the VS Code contribution (this is my first approach). How should we do for carry on this improvements. Should users vote for it?

Of course my code is just an example and this component should be modified more deeply.

Waiting for you feedback.

Regards

Gianluigi

liguori commented Jun 21, 2018

Hi @isidorn
Your solution could be a good workaround. I opened this thread as "feature request" since the debug and inspection experience should be improved for more complex scenarios.

I checked the component source code files (debugHover.css & debugHover.ts) and i noticed that the widget sizes are hard coded as:
width= 324px;

height= item number (max 18) * 18px = max 324px; (

const height = Math.min(visibleElementsCount, MAX_ELEMENTS_SHOWN) * 18;
)

Trough 'git blame' i noticed that you are the author and for sure you should know better than everyone the ground up of this component.

I tried to "play" directly with the Developer Tools and some nice bheaviour could be reached without modify the JS/TS code for a basic horizontal resizing:

.monaco-editor .debug-hover-widget .complex-value {
	width:100%;
	min-width:324px;
	
}

.debug-hover-widget .monaco-scrollable-element {
	resize: both;
	padding-bottom: 15px;
}

I am new to the VS Code contribution (this is my first approach). How should we do for carry on this improvements. Should users vote for it?

Of course my code is just an example and this component should be modified more deeply.

Waiting for you feedback.

Regards

Gianluigi

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Jun 21, 2018

Contributor

I can reopen this issue and we can treat it as a feature request. If we get more users asking for this we can discuss a potnetial pr.

Contributor

isidorn commented Jun 21, 2018

I can reopen this issue and we can treat it as a feature request. If we get more users asking for this we can discuss a potnetial pr.

@isidorn isidorn reopened this Jun 21, 2018

@isidorn isidorn added this to the Backlog milestone Jun 21, 2018

@isidorn isidorn removed their assignment Jun 21, 2018

@liguori

This comment has been minimized.

Show comment
Hide comment
@liguori

liguori Jun 21, 2018

Since you are the "core contributor" on this component, did you plan some refactoring/rengineering about it? Cause maybe we can enrich this discussion, catching some ideas.

liguori commented Jun 21, 2018

Since you are the "core contributor" on this component, did you plan some refactoring/rengineering about it? Cause maybe we can enrich this discussion, catching some ideas.

@XVincentX

This comment has been minimized.

Show comment
Hide comment
@XVincentX

XVincentX Jun 21, 2018

I support this. Even though you can click the detail arrow to have a look into the single values, it's useful to have a quick overview into the debugged object.

XVincentX commented Jun 21, 2018

I support this. Even though you can click the detail arrow to have a look into the single values, it's useful to have a quick overview into the debugged object.

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 21, 2018

Contributor

Out of scope for resizing the whole hover.
However I pushed a fix to always show a horizontal scroll bar, so the situation should at least improve a bit.

Contributor

isidorn commented Sep 21, 2018

Out of scope for resizing the whole hover.
However I pushed a fix to always show a horizontal scroll bar, so the situation should at least improve a bit.

@isidorn isidorn closed this in f696090 Sep 21, 2018

@isidorn isidorn self-assigned this Sep 21, 2018

@isidorn isidorn modified the milestones: Backlog, September 2018 Sep 21, 2018

@liguori

This comment has been minimized.

Show comment
Hide comment
@liguori

liguori Sep 23, 2018

Horizontal scroll bar would be anyway a first approach to a better user experience in debugging. Thank you.

liguori commented Sep 23, 2018

Horizontal scroll bar would be anyway a first approach to a better user experience in debugging. Thank you.

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 25, 2018

Contributor

Verifier make sure you now get the horizontal scroll bar when the content is wide in the debug hover widget

Contributor

isidorn commented Sep 25, 2018

Verifier make sure you now get the horizontal scroll bar when the content is wide in the debug hover widget

@sandy081 sandy081 removed the *out-of-scope label Sep 25, 2018

@liguori

This comment has been minimized.

Show comment
Hide comment
@liguori

liguori Sep 25, 2018

      Verifier make sure you now get the horizontal scroll bar when the content is wide in the debug hover widget

I have launched the build of the last master branch and I have verified the correct result of your commit as you can see from the screenshot below:
image

There should be at least 2 things to be improved:

  1. It should be added a bottom padding/margin (at least 10px in css classes monaco-tree-wrapper or monaco-scrollable-element of the file tree.css) in case of the horizontal scrollbar is visible since it can cover the last variable on the treeview making difficult to expand the node by clicking on the arrow because covered from the horizontal scrollbar. You can see what I am talking about taking a look on this picture:
    image

  2. When you scoll the horizontal bar until the end (maybe after having expanded different nested nodes of the three), when the mouse leave on the debug widget, since its "view state" is restored the horizontal scrollbar should return at the beginning as well. Take a look on these examples for understand what I mean:
    image

Let me know if you need more details.

Thank you

liguori commented Sep 25, 2018

      Verifier make sure you now get the horizontal scroll bar when the content is wide in the debug hover widget

I have launched the build of the last master branch and I have verified the correct result of your commit as you can see from the screenshot below:
image

There should be at least 2 things to be improved:

  1. It should be added a bottom padding/margin (at least 10px in css classes monaco-tree-wrapper or monaco-scrollable-element of the file tree.css) in case of the horizontal scrollbar is visible since it can cover the last variable on the treeview making difficult to expand the node by clicking on the arrow because covered from the horizontal scrollbar. You can see what I am talking about taking a look on this picture:
    image

  2. When you scoll the horizontal bar until the end (maybe after having expanded different nested nodes of the three), when the mouse leave on the debug widget, since its "view state" is restored the horizontal scrollbar should return at the beginning as well. Take a look on these examples for understand what I mean:
    image

Let me know if you need more details.

Thank you

@isidorn isidorn added the verified label Sep 26, 2018

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 26, 2018

Contributor

@liguori thanks for verifying -> adding verified label
As for the horizontal scroll bar being shown on top of items -> that is a general tree issue which I captured here #59452
Regarding your second point I disagree, I think it should only restart when you hide it and it shows again and that is how it happens now. Automatic scroll is not something we do through our UI and especially not for cases like this where the hover can easily be hidden.

Contributor

isidorn commented Sep 26, 2018

@liguori thanks for verifying -> adding verified label
As for the horizontal scroll bar being shown on top of items -> that is a general tree issue which I captured here #59452
Regarding your second point I disagree, I think it should only restart when you hide it and it shows again and that is how it happens now. Automatic scroll is not something we do through our UI and especially not for cases like this where the hover can easily be hidden.

@liguori

This comment has been minimized.

Show comment
Hide comment
@liguori

liguori Sep 26, 2018

@isidorn I subscribed to the #59452

Regarding my second point the use case you described is the one I am talking about (hide and show again the debug widget). I didn't notice this behavhior: once the widget is hidden, when showed again the tree is restored to the initial state but the scrollbar keep the previous scrolled position.

I will double check it and I will try to make a screencast for show you what I mean,

liguori commented Sep 26, 2018

@isidorn I subscribed to the #59452

Regarding my second point the use case you described is the one I am talking about (hide and show again the debug widget). I didn't notice this behavhior: once the widget is hidden, when showed again the tree is restored to the initial state but the scrollbar keep the previous scrolled position.

I will double check it and I will try to make a screencast for show you what I mean,

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 26, 2018

Contributor

@liguori aha I understand what you are saying. I was wrong -> the scroll position does not restart when we show the tree again.
@joaomoreno is it possible to introduce a setScrollPosition on the tree which would set the horizontal scrol also (currently it is only possible for the vertical one)

Contributor

isidorn commented Sep 26, 2018

@liguori aha I understand what you are saying. I was wrong -> the scroll position does not restart when we show the tree again.
@joaomoreno is it possible to introduce a setScrollPosition on the tree which would set the horizontal scrol also (currently it is only possible for the vertical one)

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 27, 2018

Member

I was not aware you enabled horizontal scrolling in the tree for that widget. Feel free to submit PR.

Member

joaomoreno commented Sep 27, 2018

I was not aware you enabled horizontal scrolling in the tree for that widget. Feel free to submit PR.

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 27, 2018

Contributor

Created #59546 so I tackle this next milestone

Contributor

isidorn commented Sep 27, 2018

Created #59546 so I tackle this next milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment