Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ namespace TerminalAppLocalTests
TEST_METHOD(SwapPanes);

TEST_METHOD(NextMRUTab);
TEST_METHOD(TabRowVisibilityReflectsTabCount);
TEST_METHOD(VerifyCommandPaletteTabSwitcherOrder);

TEST_METHOD(TestWindowRenameSuccessful);
Expand Down Expand Up @@ -1144,6 +1145,24 @@ namespace TerminalAppLocalTests
});
}

void TabTests::TabRowVisibilityReflectsTabCount()
{
auto page = _commonSetup();

TestOnUIThread([&page]() {
VERIFY_IS_FALSE(page->IsTabRowVisible(), L"Single tab with tabs hidden in title bar should not show the tab row.");
});

TestOnUIThread([&page]() {
NewTerminalArgs newTerminalArgs{ 1 };
page->_OpenNewTab(newTerminalArgs);
});

TestOnUIThread([&page]() {
VERIFY_IS_TRUE(page->IsTabRowVisible(), L"Multiple tabs should make the tab row visible when tabs are outside the title bar.");
});
}

void TabTests::VerifyCommandPaletteTabSwitcherOrder()
{
// This is a test for GH#8188 - we want to make sure that the order of tabs
Expand Down
6 changes: 1 addition & 5 deletions src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,7 @@ namespace winrt::TerminalApp::implementation
// - we're not in focus mode
// - we're not in full screen, or the user has enabled fullscreen tabs
// - there is more than one tab, or the user has chosen to always show tabs
const auto isVisible = !_isInFocusMode &&
(!_isFullscreen || _showTabsFullscreen) &&
(_settings.GlobalSettings().ShowTabsInTitlebar() ||
(_tabs.Size() > 1) ||
_settings.GlobalSettings().AlwaysShowTabs());
const auto isVisible = IsTabRowVisible();

if (_tabView)
{
Expand Down
14 changes: 14 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4210,6 +4210,20 @@ namespace winrt::TerminalApp::implementation
return _isAlwaysOnTop;
}

bool TerminalPage::IsTabRowVisible() const
{
if (!_settings)
{
return false;
}

return !_isInFocusMode &&
(!_isFullscreen || _showTabsFullscreen) &&
(_settings.GlobalSettings().ShowTabsInTitlebar() ||
NumberOfTabs() > 1 ||
_settings.GlobalSettings().AlwaysShowTabs());
}

// Method Description:
// - Returns true if the tab row should be visible when we're in full screen
// state.
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ namespace winrt::TerminalApp::implementation
bool FocusMode() const;
bool Fullscreen() const;
bool AlwaysOnTop() const;
bool IsTabRowVisible() const;
bool ShowTabsFullscreen() const;
void SetShowTabsFullscreen(bool newShowTabsFullscreen);
void SetFullscreen(bool);
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.idl
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ namespace TerminalApp
Boolean Fullscreen { get; };
Boolean AlwaysOnTop { get; };

Boolean IsTabRowVisible();

WindowProperties WindowProperties { get; };
void IdentifyWindow();

Expand Down
44 changes: 21 additions & 23 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1314,29 +1314,27 @@ namespace winrt::TerminalApp::implementation
winrt::Windows::Foundation::Size pixelSize = { static_cast<float>(args.Width()), static_cast<float>(args.Height()) };
const auto scale = static_cast<float>(DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel());

if (!FocusMode())
{
if (!_settings.GlobalSettings().AlwaysShowTabs())
{
// Hide the title bar = off, Always show tabs = off.
static constexpr auto titlebarHeight = 10;
pixelSize.Height += (titlebarHeight)*scale;
}
else if (!_settings.GlobalSettings().ShowTabsInTitlebar())
{
// Hide the title bar = off, Always show tabs = on.
static constexpr auto titlebarAndTabBarHeight = 40;
pixelSize.Height += (titlebarAndTabBarHeight)*scale;
}
// Hide the title bar = on, Always show tabs = on.
// In this case, we don't add any height because
// NonClientIslandWindow::GetTotalNonClientExclusiveSize() gets
// called in AppHost::_resizeWindow and it already takes title bar
// height into account. In other cases above
// IslandWindow::GetTotalNonClientExclusiveSize() is called, and it
// doesn't take the title bar height into account, so we have to do
// the calculation manually.
}
if (!FocusMode())
{
const auto tabsInTitlebar = _settings.GlobalSettings().ShowTabsInTitlebar();
const auto tabRowVisible = _root ? _root->IsTabRowVisible() :
(_settings.GlobalSettings().AlwaysShowTabs() || tabsInTitlebar);

static constexpr auto titlebarHeight = 10;
static constexpr auto titlebarAndTabBarHeight = 40;

if (tabRowVisible)
{
pixelSize.Height += titlebarAndTabBarHeight * scale;
}
else if (!tabsInTitlebar)
{
pixelSize.Height += titlebarHeight * scale;
}
Comment on lines +1326 to +1333
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhecker I think I need a truth table to work through all the logics here - I am starting to doubt my own problem-solving abilities :D

Copy link
Member

@lhecker lhecker Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this approach is correct in general. It can't be that we have to expose any WinRT properties from class A so that another class B can compute the size needed for A. A should know that by itself. But this (the way sizes are computed) is the framework we've got now and can't be easily changed.

The new boolean logic does make sense to me though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhecker @DHowett thanks for the feedback, I'll look into this deeper and try to implement simpler logic.

// When tabs are hosted in the title bar but not visible we don't
// need to adjust the size – the non-client window chrome already
// accounts for it.
}

args.Width(static_cast<int32_t>(pixelSize.Width));
args.Height(static_cast<int32_t>(pixelSize.Height));
Expand Down
Loading