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.

Sign up for GitHub

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

Jump to bottom

Fixes apply debug console font size setting to input field #91261

Merged
merged 4 commits into from
Apr 7, 2020
Merged

Fixes apply debug console font size setting to input field #91261

merged 4 commits into from
Apr 7, 2020

Conversation

jeanp413
Copy link
Contributor

This PR fixes #90714

@isidorn isidorn self-assigned this Feb 25, 2020
isidorn
isidorn reviewed Apr 6, 2020
View reviewed changes
src/vs/workbench/contrib/debug/browser/repl.ts Outdated
@@ -305,6 +305,16 @@ export class Repl extends ViewPane implements IHistoryNavigationWidget {
`;

this.tree.rerender();

this.replInput.updateOptions({
fontSize,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not take the fontFamily, lineHeight computed above? Why use debugConsole.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those values are for the inline css

  • lineHeight computed above has type string (? `${debugConsole.lineHeight}px` : '1.4em') and the one needed for updateOptions must be number
  • fontFamily computed above resolves to 'var(--monaco-monospace-font)' when the default font is requested

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thank you

@isidorn
Copy link
Contributor

isidorn commented Apr 6, 2020

@jeanp413 great PR as usual, thank you very much for creating this.
I have left a comment in the code changes. Also I tried this out and seems to work quite well, noticed one issue though: for larger font sizes the input chevron is not focused. We probably hard code the size somewhere, and the font change breaks this. Can you maybe look into improving the centering of the chevron on the left?

Screenshot 2020-04-06 at 13 03 09

@isidorn isidorn added this to the April 2020 milestone Apr 6, 2020
@isidorn isidorn added the debug Debug viewlet, configurations, breakpoints, adapter issues label Apr 6, 2020
@isidorn
Copy link
Contributor

isidorn commented Apr 7, 2020

@jeanp413 your comment makes sense. Once we resolve the centering of the chevron (if possible) I would be open to merge in this PR.

@jeanp413
Copy link
Contributor Author

jeanp413 commented Apr 7, 2020

Pushed some changes to center de input chevron, let me know if the css is ok

@isidorn
Copy link
Contributor

isidorn commented Apr 7, 2020

@jeanp413 Yeah, css looks great. Let's merge this.
Thanks a lot 🍻 ☀️

@isidorn isidorn merged commit 8b919e4 into microsoft:master Apr 7, 2020
@jeanp413 jeanp413 deleted the fix-90714 branch April 7, 2020 14:10
@janosh janosh mentioned this pull request Apr 8, 2020
Closed
@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@isidorn isidorn isidorn left review comments

@AdrianDeAnda AdrianDeAnda AdrianDeAnda approved these changes

Assignees

@isidorn isidorn

Labels
debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Milestone
April 2020
Development

Successfully merging this pull request may close these issues.

Apply debug console font size setting to input field
3 participants
@jeanp413 @isidorn @AdrianDeAnda