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

(cherry picked from commit 21a9c55)
Service-Card-Id: 86209056
Service-Version: 1.16
  • Loading branch information
jonathan-meier authored and DHowett committed Oct 13, 2022
1 parent c82a577 commit 926c8e0
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 926c8e0

Please sign in to comment.