-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
fix: Param helper hover getting cut off at bottom #112019
Conversation
asyncRenderCallback: () => { | ||
this.domNodes?.scrollbar.scanDomNode(); | ||
} | ||
})); |
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 line and the lines surrounding lines are a duplicate of line 228-232. Other than the dom element target (documentation or dom variable). Should we refactor to bring these into the same place e.g.
if (typeof signature.documentation === 'string') {
dom.append(this.domNodes.docs, $('p', {}, signature.documentation));
} else {
const renderedContents = this.renderDisposeables.add(this.markdownRenderer.render(signature.documentation));
const renderedContents = this.renderDisposeables.add(this.markdownRenderer.render(signature.documentation, {
asyncRenderCallback: () => {
this.domNodes?.scrollbar.scanDomNode();
}
}));
renderedContents.element.classList.add('markdown-docs');
const target = activeParameter?.documentation ?? dom;
target.append(this.domNodes.docs, renderedContents.element);
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.
just like this?
private renderMarkdownDocs(markdown: IMarkdownString | undefined): IMarkdownRenderResult {
const renderedContents = this.renderDisposeables.add(this.markdownRenderer.render(markdown, {
asyncRenderCallback: () => {
this.domNodes?.scrollbar.scanDomNode();
}
}));
renderedContents.element.classList.add('markdown-docs');
return renderedContents;
}
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.
LGTM
3a28190
to
b2d1228
Compare
@joaomoreno Can you help take a look at this PR |
Thanks! Should be available in the first VS Code 1.53 insiders builds |
This PR fixes #105422