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

wpf: base margin height off Y dpi, not X dpi #8039

Conversation

ScriptKat
Copy link
Contributor

@ScriptKat ScriptKat commented Oct 25, 2020

This PR resolves an issue I observed in
Microsoft.Terminal.Wpf.TerminalControl.CalculateMargins(). Specifically,
on line 194 in the project. In this example, the line: height = controlSize.Height - (this.TerminalRendererSize.Height / dpiScale.DpiScaleX); is associating the height margin with
dpiScale.DpiScaleX instead of dpiScale.DpiScaleY. This PR changes the
association to DpiScaleY.

Closes #8038

… on the height margin from dpiScale.DpiScaleX to dpiScale.DpiScaleY in Microsoft.Terminal.Wpf.TerminalControl.CalculateMargins().
@ScriptKat
Copy link
Contributor Author

It looks like the Terminal CI builds failed while trying to restore packages.. is there any way to re-execute these tests?

@DHowett
Copy link
Member

DHowett commented Oct 26, 2020

/azp run

@DHowett
Copy link
Member

DHowett commented Oct 26, 2020

Sorry -- our CI can be flaky at times 😄

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 26, 2020
@ghost
Copy link

ghost commented Oct 26, 2020

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@DHowett
Copy link
Member

DHowett commented Oct 26, 2020

You can ignore the Linter.

@DHowett DHowett added the Needs-Second It's a PR that needs another sign-off label Oct 26, 2020
@DHowett DHowett changed the title Change margin height variable to be constrained by dpi scale Y instead of dpi Scale X wpf: base margin height off Y dpi, not X dpi Oct 26, 2020
@ghost ghost requested a review from PankajBhojwani October 26, 2020 20:52
@ghost ghost merged commit c095a67 into microsoft:main Oct 26, 2020
DHowett pushed a commit that referenced this pull request Oct 28, 2020
This PR resolves an issue I observed in
Microsoft.Terminal.Wpf.TerminalControl.CalculateMargins(). Specifically,
on line 194 in the project. In this example, the line: `height =
controlSize.Height - (this.TerminalRendererSize.Height /
dpiScale.DpiScaleX);` is associating the height margin with
dpiScale.DpiScaleX instead of dpiScale.DpiScaleY. This PR changes the
association to DpiScaleY.

Closes #8038

(cherry picked from commit c095a67)
@ghost
Copy link

ghost commented Nov 11, 2020

🎉Windows Terminal v1.4.3141.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Nov 11, 2020

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met hacktoberfest-accepted Needs-Second It's a PR that needs another sign-off
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A margin variable is being constrained by dpi scale X instead of dpi Scale Y
3 participants