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

Another hole in the Terminal when opening a dialog #12447

Closed
zadjii-msft opened this issue Feb 9, 2022 · 11 comments · Fixed by #12517
Closed

Another hole in the Terminal when opening a dialog #12447

zadjii-msft opened this issue Feb 9, 2022 · 11 comments · Fixed by #12517
Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes.
Milestone
Terminal v1.14

Comments

@zadjii-msft
Copy link
Member

zadjii-msft commented Feb 9, 2022 •
edited
Loading

I forgot to report a bug related to this.

If I open Settings and then also About, and then click the Close button, part of the screen is shifted.

So I thought this behavior was a bug, not a feature.

image2

Originally posted by @akihironagai in #12396 (comment)

ALSO from #12455

Windows Terminal version

1.13.10336.0

Steps to reproduce

  1. copy a bunch of text from another where
  2. paste into vim in Terminal
  3. a paste prompt pop out, asking me really going to pasting these bunch texts, and bug happens now.

Actual Behavior

about one quarter parts above there in the window "client area" disappeared, and that area becomes totally transparent , but mouse actions cannot go through, i.e. cannot click items back there.

the active area of terminal also shrinked, if I'm trying to cls, the prompt line will be in the top of the shrinked area, not disappeared instead.

the screenshot: scrnshot3 scrnshot
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Feb 9, 2022
@zadjii-msft
Copy link
Member Author

Another instance of #12208? but with the close tabs warning?

@zadjii-msft zadjii-msft added Area-Settings UI Anything specific to the SUI Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Feb 9, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.14 milestone Feb 9, 2022
@zadjii-msft zadjii-msft added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Feb 9, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 9, 2022
This was referenced Feb 9, 2022
Closed
Closed
@zadjii-msft zadjii-msft added Priority-1 A description (P1) and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-2 A description (P2) labels Feb 10, 2022
@epiciskandar

This comment was marked as off-topic.

Sign in to view
@epiciskandar

This comment was marked as off-topic.

Sign in to view
@zadjii-msft
Copy link
Member Author

@epiciskandar That's actually #2380

@zadjii-msft zadjii-msft added the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label Feb 14, 2022
@PankajBhojwani
Copy link
Contributor

Doing some investigation into this - my initial suspicion is that this has something to do with trying to open a dialog when there's already another dialog open. Here are the other ways I've managed to repro this:

  • Opening the 'about' dialog when there's more than one tab open (not necessarily the settings tab), and then clicking the close button (which should cause the 'confirm close all tabs' dialog to show up, but instead causes this issue)
  • Doing a multi-line paste (causing the 'warn multi line paste' dialog to show up), then clicking the close button when there's multiple tabs open

@PankajBhojwani PankajBhojwani mentioned this issue Feb 17, 2022
Merged
6 tasks
@ghost ghost added the In-PR This issue has a related PR label Feb 17, 2022
@ghost ghost closed this as completed in #12517 Feb 18, 2022
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Feb 18, 2022
ghost pushed a commit that referenced this issue Feb 18, 2022
@PankajBhojwani
No longer load content dialogs when there is already one being shown (#…
788d33c 
…12517)

## Summary of the Pull Request
Somehow, the controls v2 update caused an issue where if you as much as _load_ a content dialog when there's already one open, we get holes in the terminal window (#12447)

This commit introduces logic to `TerminalPage` to check whether there is a content dialog open before we try to load another one. 

## PR Checklist
* [x] Closes #12447 
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I work here

## Validation Steps Performed
Can no longer repro #12447
@zadjii-msft zadjii-msft mentioned this issue Mar 1, 2022
Closed
DHowett pushed a commit that referenced this issue Mar 10, 2022
@PankajBhojwani @DHowett
No longer load content dialogs when there is already one being shown (#…
c4266cc 
…12517)

## Summary of the Pull Request
Somehow, the controls v2 update caused an issue where if you as much as _load_ a content dialog when there's already one open, we get holes in the terminal window (#12447)

This commit introduces logic to `TerminalPage` to check whether there is a content dialog open before we try to load another one.

## PR Checklist
* [x] Closes #12447
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I work here

## Validation Steps Performed
Can no longer repro #12447

(cherry picked from commit 788d33c)
zadjii-msft pushed a commit that referenced this issue Mar 10, 2022
@PankajBhojwani @zadjii-msft
No longer load content dialogs when there is already one being shown (#…
f94da4f 
…12517)

Somehow, the controls v2 update caused an issue where if you as much as _load_ a content dialog when there's already one open, we get holes in the terminal window (#12447)

This commit introduces logic to `TerminalPage` to check whether there is a content dialog open before we try to load another one.

* [x] Closes #12447
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I work here

Can no longer repro #12447
@zadjii-msft zadjii-msft mentioned this issue Mar 11, 2022
Closed
@ghost
Copy link

ghost commented Mar 25, 2022

🎉This issue was addressed in #12517, which has now been successfully released as Windows Terminal Preview v1.13.1073.:tada:

Handy links:

@jnunderwood
Copy link

Hello. I am now using Windows Terminal Preview Version: 1.13.10734.0 (Windows 11). Unfortunately, I am still seeing this behavior when I paste text larger than 5KiB.

This was referenced Apr 6, 2022
Closed
Merged
ghost pushed a commit that referenced this issue Apr 6, 2022
@zadjii-msft
Manually put our ContentDialogs in the tab content, rather than above (…
c4e5ebf 
…#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
DHowett pushed a commit that referenced this issue Apr 6, 2022
@zadjii-msft @DHowett
Manually put our ContentDialogs in the tab content, rather than above (…
d0dfcad 
…#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
DHowett pushed a commit that referenced this issue Apr 6, 2022
@zadjii-msft @DHowett
Manually put our ContentDialogs in the tab content, rather than above (…
173de03 
…#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
@epiciskandar
Copy link

Unfortunately, I am still seeing this behavior

same here, it's not really being fixed

@zadjii-msft
Copy link
Member Author

@epiciskandar Which version are you on? There's been 3 different attempts to fix this so far, all in different versions, so you might not have the very latest fix yet.

@epiciskandar
Copy link

@epiciskandar Which version are you on? There's been 3 different attempts to fix this so far, all in different versions, so you might not have the very latest fix yet.

Windows Terminal Preview
Version: 1.13.10733.0

Microsoft Windows [Version 10.0.19044.1620]

@DHowett
Copy link
Member

DHowett commented Apr 11, 2022

The fix will be released with the "1098" series of servicing updates. That will be 1.12.10982/3/4 and 1.13.10983/4

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes.
Projects
None yet
Milestone
Terminal v1.14
Development

Successfully merging a pull request may close this issue.

No longer load content dialogs when there is already one being shown microsoft/terminal
5 participants
@DHowett @epiciskandar @jnunderwood @zadjii-msft @PankajBhojwani