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

A very long title causes only half of the progress ring to be visible #8910

Closed
Tracked by #6700
skyline75489 opened this issue Jan 27, 2021 · 2 comments · Fixed by #14167
Closed
Tracked by #6700

A very long title causes only half of the progress ring to be visible #8910

skyline75489 opened this issue Jan 27, 2021 · 2 comments · Fixed by #14167
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@skyline75489
Copy link
Collaborator

skyline75489 commented Jan 27, 2021

Environment

Windows build number: [run `[Environment]::OSVersion` for powershell, or `ver` for cmd] 19042
Windows Terminal version (if applicable): 1.6 preview

Any other software?

Steps to reproduce

I was trying the bcz.cmd and found only half of the progress ring is shown.

Here's my profile:

            {
                "name" : "Developer Command Prompt for VS 2019",
                "commandline" : "cmd.exe /k \"C:/\"Program Files (x86)\"/\"Microsoft Visual Studio\"/2019/Enterprise/Common7/Tools/VsDevCmd.bat\"",
                "startingDirectory" : "%USERPROFILE%",
                "colorScheme": "Chalk"
            }

Expected behavior

The ring. The whole ring. The HALO.

Actual behavior

image


I'm only seeing this in the VS cmd profile. All other profiles work fine:

image

@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 Jan 27, 2021
@zadjii-msft
Copy link
Member

Okay, I'm testing this out. It doesn't look like it's the fallback icon that causes this. I think it's the title length. If you set the tab title to something really long, like Developer Command Prompt A really really really long title, then it seems to push the spinner halfway into oblivion.

That's real bizarre, because there's definitely space for it, but the tab view is still cropping it. Weird.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jan 27, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 27, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Jan 27, 2021
@zadjii-msft zadjii-msft changed the title I'm only seeing half of the progress ring in VS cmd profile A very long title causes only half of the progress ring to be visible Jan 27, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 28, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal v2.0, 22H2 Jan 4, 2022
@ghost ghost added the In-PR This issue has a related PR label Oct 9, 2022
@ghost ghost closed this as completed in #14167 Oct 10, 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 Oct 10, 2022
ghost pushed a commit that referenced this issue Oct 10, 2022
It turns out that the negative margin for the progress ring is causing
the clipping in case the tab title gets too long:

https://github.com/microsoft/terminal/blob/43dbbd590fa4b46c37e9970415f8150d3c399598/src/cascadia/TerminalApp/TabHeaderControl.xaml#L18-L27

The negative margin was introduced in #8113 because the progress ring is
supposed to replace the tab icon but the `TabView` still reserves space
even if no icon is set (see
#8133 (comment)).
However, it is not actually the `TabView` reserving space even when
there is no icon, but a workaround for a crash in the
`IconPathConverter` that returns a `BitmapIconSource` with a `nullptr`
source instead of a `nullptr` `IconSource`:

https://github.com/microsoft/terminal/blob/43dbbd590fa4b46c37e9970415f8150d3c399598/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp#L143-L154

The workaround in `IconPathConverter` could probably be removed as I did
not find any instance where it is still used in a way that could trigger
the mentioned crash, but I did not dare to just remove it as I do not
know enough about the code by far. Hence, I opted to just locally
instantiate the `IconSource` with a `nullptr` directly in `TerminalTab`.

Fixes #8910
DHowett pushed a commit that referenced this issue Oct 13, 2022
It turns out that the negative margin for the progress ring is causing
the clipping in case the tab title gets too long:

https://github.com/microsoft/terminal/blob/43dbbd590fa4b46c37e9970415f8150d3c399598/src/cascadia/TerminalApp/TabHeaderControl.xaml#L18-L27

The negative margin was introduced in #8113 because the progress ring is
supposed to replace the tab icon but the `TabView` still reserves space
even if no icon is set (see
#8133 (comment)).
However, it is not actually the `TabView` reserving space even when
there is no icon, but a workaround for a crash in the
`IconPathConverter` that returns a `BitmapIconSource` with a `nullptr`
source instead of a `nullptr` `IconSource`:

https://github.com/microsoft/terminal/blob/43dbbd590fa4b46c37e9970415f8150d3c399598/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp#L143-L154

The workaround in `IconPathConverter` could probably be removed as I did
not find any instance where it is still used in a way that could trigger
the mentioned crash, but I did not dare to just remove it as I do not
know enough about the code by far. Hence, I opted to just locally
instantiate the `IconSource` with a `nullptr` directly in `TerminalTab`.

Fixes #8910

(cherry picked from commit 21a9c55)
Service-Card-Id: 86209056
Service-Version: 1.16
@ghost
Copy link

ghost commented Dec 14, 2022

🎉This issue was addressed in #14167, which has now been successfully released as Windows Terminal Preview v1.16.3463.0 and v1.16.3464.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants