Skip to content

Commit

Permalink
Fix clipped progress ring in tab when tab title is too long (#14167)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jonathan-meier committed Oct 10, 2022
1 parent 43dbbd5 commit 21a9c55
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 7 deletions.
7 changes: 1 addition & 6 deletions src/cascadia/TerminalApp/TabHeaderControl.xaml
Expand Up @@ -20,16 +20,11 @@
Height="15"
MinWidth="0"
MinHeight="0"
Margin="-7.5,0,8,0"
Margin="3,0,8,0"
IsActive="{x:Bind TabStatus.IsProgressRingActive, Mode=OneWay}"
IsIndeterminate="{x:Bind TabStatus.IsProgressRingIndeterminate, Mode=OneWay}"
Visibility="{x:Bind TabStatus.IsProgressRingActive, Mode=OneWay}"
Value="{x:Bind TabStatus.ProgressValue, Mode=OneWay}" />
<!--
We want the progress ring to 'replace' the tab icon, but we don't have control
over the tab icon here (the tab view item does) - so we hide the tab icon there
and use a negative margin for the progress ring here to put it where the icon would be
-->
<FontIcon x:Name="HeaderBellIndicator"
Margin="0,0,8,0"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/TerminalTab.cpp
Expand Up @@ -15,6 +15,7 @@ using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Core;
using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::UI::Xaml::Controls;
using namespace winrt::Windows::System;

namespace winrt
Expand Down Expand Up @@ -316,7 +317,7 @@ namespace winrt::TerminalApp::implementation
if (hide)
{
Icon({});
TabViewItem().IconSource(IconPathConverter::IconSourceMUX({}));
TabViewItem().IconSource(IconSource{ nullptr });
}
else
{
Expand Down

0 comments on commit 21a9c55

Please sign in to comment.