Don't open a hole in the terminal window when pasting by zadjii-msft · Pull Request #12208 · microsoft/terminal

Mike Griese (zadjii-msft) · 2022-01-20T18:20:00Z

Turns out, this bug only repros in Controls version 2. I'm not sure why, but it didn't repro only on main. So this fix does nothing until #11720 merges.

This PR prevents us from setting properties on the paste warning dialog unless we actually need to paste. 5f9c551 proves that settings these properties is what would cause the bug in the first place.

I went a step further and cleaned this up a bit. This was always a little weird, having to get the BracketedPasteEnabled for the active control on the UI thread before we actually display the warning. In the post-#5000 future where going back to the control like this would be a x-proc hop, I figured I should just skip that entirely and plumb the BracketedPaste state out in the initial request.

Closes Pasting multiple lines in bracketed paste mode creates a giant transparent hole #12202
I work here
No tests, but there's not a great place for a test like this
Doesn't affect docs

See also: #12241 which would introduce #12202 on its own.

… though...

James Holderness (j4james) · 2022-01-21T00:20:36Z

I'm assuming the "whole" in your PR title was meant to be "hole".

Mike Griese (zadjii-msft) · 2022-01-21T11:59:52Z

I'm assuming the "whole" in your PR title was meant to be "hole".

yep, yea. Thanks 😅

Dustin L. Howett (DHowett)

I don't totally understand how this fixes it . . . what if bracketed paste ISN'T enabled?

I am having trouble reproducing this issue to begin with, so I'm doubly baffled.

Mike Griese (zadjii-msft) · 2022-01-25T18:02:18Z

I don't totally understand how this fixes it . . . what if bracketed paste ISN'T enabled?

I am having trouble reproducing this issue to begin with, so I'm doubly baffled.

Honestly, it makes no sense. Seemingly, the issue arises from doing

                FindName(L"MultiLinePasteDialog").try_as<WUX::Controls::ContentDialog>();
                ClipboardText().Text(text);
                ClipboardContentScrollViewer().ScrollToVerticalOffset(0);

Without the call to

warningResult = co_await _ShowMultiLinePasteWarningDialog()

I couldn't fix the issue by commenting out any single line of the first three. I could however repro even w/o Bracketed Paste just by commenting out the co_await line.

Actually, just calling FindName(L"MultiLinePasteDialog").try_as<WUX::Controls::ContentDialog>(); WITHOUT showing the dialog seems to cause this. I honestly don't know more than that.

Deleted user (ghost) · 2022-01-27T23:56:15Z

Hello Dustin L. Howett (@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.

…#12840) After switching to ControlsV2, it seems that delay-loading a dialog causes the ContentDialog to be assigned a Height equal to it's content size. If we DON'T assign the ContentDialog a Row, I believe it's assigned Row 0 by default. So, when the dialog gets opened, the dialog seemingly causes a giant hole to appear in the body of the app. Assigning all the dialogs to Row 2 (where the rest of the content is) makes the "hole" appear in the same space as the rest of the TabContent, fixing the issue. Note that the actual content in a content dialog gets parented to the PopupRoot, so it actually always appeared in the correct place, it's just this weird hole that appeared in Row 0. * [x] Closes #12775 * See also: * #12202 was fixed by #12208 * #12447 was fixed by #12517 * [x] Tested manually * [x] Reverts #12625 * [x] Reverts #12517

…#12840) After switching to ControlsV2, it seems that delay-loading a dialog causes the ContentDialog to be assigned a Height equal to it's content size. If we DON'T assign the ContentDialog a Row, I believe it's assigned Row 0 by default. So, when the dialog gets opened, the dialog seemingly causes a giant hole to appear in the body of the app. Assigning all the dialogs to Row 2 (where the rest of the content is) makes the "hole" appear in the same space as the rest of the TabContent, fixing the issue. Note that the actual content in a content dialog gets parented to the PopupRoot, so it actually always appeared in the correct place, it's just this weird hole that appeared in Row 0. * [x] Closes #12775 * See also: * #12202 was fixed by #12208 * #12447 was fixed by #12517 * [x] Tested manually * [x] Reverts #12625 * [x] Reverts #12517 (cherry picked from commit c4e5ebf) Service-Card-Id: 80266902 Service-Version: 1.12

…#12840) After switching to ControlsV2, it seems that delay-loading a dialog causes the ContentDialog to be assigned a Height equal to it's content size. If we DON'T assign the ContentDialog a Row, I believe it's assigned Row 0 by default. So, when the dialog gets opened, the dialog seemingly causes a giant hole to appear in the body of the app. Assigning all the dialogs to Row 2 (where the rest of the content is) makes the "hole" appear in the same space as the rest of the TabContent, fixing the issue. Note that the actual content in a content dialog gets parented to the PopupRoot, so it actually always appeared in the correct place, it's just this weird hole that appeared in Row 0. * [x] Closes #12775 * See also: * #12202 was fixed by #12208 * #12447 was fixed by #12517 * [x] Tested manually * [x] Reverts #12625 * [x] Reverts #12517 (cherry picked from commit c4e5ebf) Service-Card-Id: 80266903 Service-Version: 1.13

Mike Griese (zadjii-msft) added 2 commits January 20, 2022 11:33

this fixes it in a braindead way. Gonna try and clear some of this up…

5f9c551

… though...

much cleaner

685ea89

Mike Griese (zadjii-msft) changed the title ~~Don't open a whole in the terminal window when pasting~~ Don't open a hole in the terminal window when pasting Jan 21, 2022

Dustin L. Howett (DHowett) reviewed Jan 21, 2022

View reviewed changes

Leonard Hecker (lhecker) approved these changes Jan 24, 2022

View reviewed changes

Leonard Hecker (lhecker) self-requested a review January 24, 2022 02:55

Dustin L. Howett (DHowett) approved these changes Jan 27, 2022

View reviewed changes

Dustin L. Howett (DHowett) added the Needs-Second It's a PR that needs another sign-off label Jan 27, 2022

ghost requested a review from Michael Niksa (miniksa) January 27, 2022 23:56

Dustin L. Howett (DHowett) added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 27, 2022

ghost requested review from PankajBhojwani, Carlos Zamora (carlos-zamora) and Leon Liang (leonMSFT) January 27, 2022 23:56

ghost merged commit 95770ed into main Jan 27, 2022

ghost deleted the dev/migrie/b/12202-gaping-holes-when-pasting branch January 27, 2022 23:56

mailinglists35 mentioned this pull request Feb 6, 2022

Update changed tab colors so I can no longer figure out which one is active 🤬 #12398

Closed

Mike Griese (zadjii-msft) mentioned this pull request Feb 9, 2022

Another hole in the Terminal when opening a dialog #12447

Closed

This was referenced Apr 6, 2022

UX REGRESSION - Pasting 5kb of text causes half of the terminal to become transparent #12775

Closed

Manually put our ContentDialogs in the tab content, rather than above #12840

Merged

This pull request was closed.

Navigation Menu

Repository navigation

Don't open a hole in the terminal window when pasting - #12208

Don't open a hole in the terminal window when pasting#12208
2 commits merged into
mainfrom
dev/migrie/b/12202-gaping-holes-when-pasting

Mike Griese (zadjii-msft) commented Jan 20, 2022 •

edited

Loading

Uh oh!

James Holderness (j4james) commented Jan 21, 2022

Uh oh!

Mike Griese (zadjii-msft) commented Jan 21, 2022

Uh oh!

Dustin L. Howett (DHowett) left a comment

Uh oh!

Mike Griese (zadjii-msft) commented Jan 25, 2022

Uh oh!

Deleted user (ghost) commented Jan 27, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Mike Griese (zadjii-msft) commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

James Holderness (j4james) commented Jan 21, 2022

Uh oh!

Mike Griese (zadjii-msft) commented Jan 21, 2022

Uh oh!

Dustin L. Howett (DHowett) left a comment

Choose a reason for hiding this comment

Uh oh!

Mike Griese (zadjii-msft) commented Jan 25, 2022

Uh oh!

Deleted user (ghost) commented Jan 27, 2022

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.

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mike Griese (zadjii-msft) commented Jan 20, 2022 •

edited

Loading

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.