-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
GH-8361: Fixed editor resizing issue in Output. #8362
Conversation
Closes #8361. Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@@ -147,6 +147,11 @@ export class OutputWidget extends BaseWidget implements StatefulWidget { | |||
} | |||
} | |||
|
|||
protected onAfterShow(msg: Message): void { | |||
super.onAfterShow(msg); | |||
this.onResize(Widget.ResizeMessage.UnknownSize); // Triggers an editor widget resize. (#8361) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder what is the difference to explicit this.editor.refresh();
.
In EditorWidget
we do it, we do it as well for onAfterAttach
to resize it if a widget gets moved between areas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I started by calling editor.refresh()
but it did not fix the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.autoSizing
is false
in the monaco editor, so calling refresh
is a NOOP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turned out, calling editor.refresh()
does nothing at all when invoked from EditorWidget#onAfterShow
. autoSizing
is always false
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In
EditorWidget
we do it, we do it as well foronAfterAttach
to resize it if a widget gets moved between areas.
I have tried opening, maximizing, and DNDing editors while having breakpoints on both onAfterAttach
and onAfterShow
in the editor widget:
this.editor.refresh(); |
this.editor.refresh(); |
Neither of them has any effect as the underlying monaco editor's autoresize
is a noop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, somehow related to the old issue? microsoft/monaco-editor#794
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirmed that the bug is present on master
, and that the following changes resolves the issue using the reproduction steps 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's go with it
Closes #8361.
Signed-off-by: Akos Kitta kittaakos@typefox.io
What it does
Fixes the editor resizing issue in the Output view.
How to test
API sample
Output channel is visible.Before:
After:
Review checklist
Reminder for reviewers