Calculate initial height properly#8584
Conversation
zadjii-msft
left a comment
There was a problem hiding this comment.
Okay, I don't think this is the right solution. First and foremost, I think we should already be handling this case in the if/else if below this. That's probably the thing that's returning the wrong value.
Secondly - TermControl shouldn't need to know anything about the presence of tabs or not. It should be TerminalPage/AppLogic/TerminalApp responsibility to account for the size of the tabs, not the control. The control should be agnostic of whoever's hosting it (it doesn't care if the hosting application has tabs or not)
|
I tested all four true false cases, and it worked. However I'm not sure this was the right solution. |
zadjii-msft
left a comment
There was a problem hiding this comment.
I can't believe all this time, it said Width and not Height 🤦♂️
I'm only blocking over updating the comment, but yea the fix seems good to me.
| // For whatever reason, there's about 6px of unaccounted-for space | ||
| // in the application. I couldn't tell you where these 6px are | ||
| // coming from, but they need to be included in this math. |
There was a problem hiding this comment.
| // For whatever reason, there's about 6px of unaccounted-for space | |
| // in the application. I couldn't tell you where these 6px are | |
| // coming from, but they need to be included in this math. | |
| // For whatever reason, there's about 10px of unaccounted-for space | |
| // in the application. I couldn't tell you where these 10px are | |
| // coming from, but they need to be included in this math. |
|
@zadjii-msft I fixed the comment. |
|
Wow. I can't believe we were calculating the width instead of the height. Thank you! |
|
Hello @DHowett! Because this pull request has the 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 (
|
|
🎉 Handy links: |
|
🎉 Handy links: |
Closes #8527