Files
DodoSSH/tests/DodoSSH.Client.App.Layout.Tests/TerminalTabsTests.cs
T
jaap-janandClaude Opus 5 359087f1ce
ci / build and test (push) Successful in 1m12s
ci / android head (push) Failing after 5s
ci / api image (push) Successful in 27s
Give the tabs their hover back, and the plus the shape its comment claims
Two things v2 broke in the last commit, both found by reading resolved brushes rather than markup, and both
the same mistake: Avalonia has no specificity, so the later declaration wins, and a rule that restated the
base instead of excepting from it went in below the rules it was supposed to be underneath.

Making a tab a pill gave it a Background of its own, which the flat tab it replaced never had. That one
detail moved where the hover has to live. `Button.flat:pointerover` is declared far above and had been
supplying it; the moment `Button.tab`'s template rule set a Background, it won, and every tab in the strip
stopped answering the pointer. Silently — a tab that no longer lights is not a crash and not a layout
change, and nothing in a suite that measures heights and reachability can see it.

The `+` lost more than that. It sits below as an exception — no outline, because it is not one of the
things being chosen between — and a `.tab` rule declared after it was overriding the exception itself. It
drew as a filled, outlined pill identical to a tab, contradicting the comment directly above it.

So the base pill and its hover come first now and the exceptions follow, which is the order the rest of
this file already uses and the order the Border.rowmark note further down was written about. The `+` clears
the fill as well as the border, because an exception to a rule that sets both has to say both.

The titlebar's search box had the same shape of error in geometry rather than colour. The design draws it
at exactly 380 and centred, and stating that as a Width on the inner Border is what made it wrong: the
button around it is free to shrink when the account name or the vault chip beside it is long, and a Border
that will not shrink with it arranges outside its own parent — over the name on one side and over the
window buttons on the other. MaxWidth on a stretching button gives the same 380 whenever there is room and
gives way when there is not.

The regression test is the point of this commit rather than an afterthought. It is the only test in that
suite that reads a brush, and the gap it fills is exactly the one these two went through: everything else
measures rectangles. It hovers a tab through the real input path and asserts the fill changes, then asserts
the `+` is neither filled like a tab nor outlined like one. Checked against the broken ordering before
being kept — it fails there and passes here, which is the only thing that makes a regression test worth
committing.

Verified by the whole suite: 1310 tests over nineteen projects, none failing, the layout suite now 70
cases. Both heads build. Still nothing seen on a display.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AZE3u99BNt6LzgTC5jhbz2
2026-08-02 19:54:37 +02:00

359 lines
14 KiB
C#

using Avalonia;
using Avalonia.Controls;
using Avalonia.Controls.Presenters;
using Avalonia.Headless;
using Avalonia.Input;
using Avalonia.Media;
using Avalonia.VisualTree;
using DodoSSH.Client.App.Views;
using DodoSSH.Client.Session;
using DodoSSH.Client.Shell.ViewModels;
using DodoSSH.Client.Ssh;
using DodoSSH.Client.Storage;
using DodoSSH.Client.Terminal;
using NSubstitute;
namespace DodoSSH.Client.App.Layout.Tests;
/// <summary>
/// How the tab strip answers a pointer.
/// </summary>
/// <remarks>
/// <para>
/// The strip spans every screen now, so it is chrome a user is in contact with all day rather than one
/// column of the hosts screen. What that earns it is the gestures every other tabbed application has — a
/// middle click that closes, a cross inside the tab rather than beside it, a button that opens another — and
/// what those need is a suite, because all three are pointer behaviour and none of it is expressible as a
/// binding.
/// </para>
/// <para>
/// A <c>UserControl</c> in a bare window, for the reason the palette's suite is one:
/// <see cref="LayoutHarnessTests.WhyTheWindowItselfIsNeverShown"/>. No vault and no session — the strip
/// binds only to the shell's tab list, and tabs are shell state that outlives the vault that opened them, so
/// they can be put there directly. Closing one asks the workspace to end a session it has never heard of,
/// which the workspace answers by returning: that is the same path a real close takes, minus a shell.
/// </para>
/// </remarks>
public sealed class TerminalTabsTests : IAsyncLifetime
{
private ClientCacheFactory caches = null!;
private TerminalWorkspace workspace = null!;
private MainWindowViewModel shell = null!;
private static CancellationToken Token => TestContext.Current.CancellationToken;
/// <inheritdoc />
public ValueTask InitializeAsync()
{
caches = ClientCacheFactory.ForMemory($"tabs-{Guid.CreateVersion7():N}");
workspace = new TerminalWorkspace(
new InMemoryTerminalAssetProvider(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)),
Substitute.For<ISshConnectionFactory>(),
TimeProvider.System);
shell = new MainWindowViewModel(
ClientPaths.Default,
caches,
workspace,
new VaultKnownHostStore(),
Substitute.For<IDeviceKeyStore>(),
(_, _) => throw new NotSupportedException("nothing here signs in"),
TimeProvider.System,
Substitute.For<ISftpSessionFactory>())
{
// The only state the strip is ever interactive in. Assigned rather than reached through an
// enrollment, which would be an Argon2 pass for no extra coverage — nothing here reads the vault.
State = ShellState.Unlocked,
};
return ValueTask.CompletedTask;
}
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
await shell.DisposeAsync();
await workspace.DisposeAsync();
caches.Dispose();
}
/// <remarks>
/// The gesture this rework is for. Middle-clicking a tab is how every browser and every terminal closes
/// one, and the strip answered nothing but a left click before.
/// </remarks>
[Fact]
public async Task AMiddleClickOnATabClosesThatTab()
{
await OnTheStripAsync((strip, window) =>
{
var doomed = shell.Tabs[0];
var survivor = shell.Tabs[1];
window.MouseDown(Centre(TabButton(strip, doomed), window), MouseButton.Middle);
shell.Tabs.ShouldHaveSingleItem().ShouldBe(survivor);
});
}
/// <remarks>
/// The other half of the rule, and the reason the handler is on the tab's own template root rather than
/// on the strip: a middle click on the chrome between the last tab and the edge of the window must not
/// close anything. Wiring it on the strip and testing what was underneath the pointer would have been
/// the same feature with a way to get it wrong.
/// </remarks>
[Fact]
public async Task AMiddleClickOnTheStripBackgroundClosesNothing()
{
await OnTheStripAsync((strip, window) =>
{
// Well right of two short tabs and the button after them, and inside the strip's own height.
window.MouseDown(new Point(700, 17), MouseButton.Middle);
shell.Tabs.Count.ShouldBe(2);
});
}
[Fact]
public async Task AMiddleClickOnTheButtonThatOpensAConnectionClosesNothing()
{
await OnTheStripAsync((strip, window) =>
{
window.MouseDown(Centre(PlusButton(strip), window), MouseButton.Middle);
shell.Tabs.Count.ShouldBe(2);
shell.IsSearching.ShouldBeFalse("a middle click is not how the palette opens either");
});
}
/// <remarks>
/// The cross is inside the tab, so a middle click on it bubbles out to the tab's handler as well. One
/// close, not two: the second would take the neighbour, which is the tab the user was aiming to keep.
/// </remarks>
[Fact]
public async Task AMiddleClickOnTheCrossClosesExactlyOneTab()
{
await OnTheStripAsync((strip, window) =>
{
var survivor = shell.Tabs[1];
window.MouseDown(Centre(CloseButton(strip, shell.Tabs[0]), window), MouseButton.Middle);
shell.Tabs.ShouldHaveSingleItem().ShouldBe(survivor);
});
}
/// <remarks>
/// The one assumption the nested-button template makes, stated as a test. Avalonia's
/// <c>Button.OnPointerPressed</c> takes the capture and marks a left press handled, so the cross does
/// not also reach the tab underneath it — which would select a tab on its way out and leave the
/// terminal switching to something that is about to disappear.
/// </remarks>
[Fact]
public async Task ALeftClickOnTheCrossClosesTheTabAndDoesNotSelectIt()
{
await OnTheStripAsync((strip, window) =>
{
var doomed = shell.Tabs[0];
var survivor = shell.Tabs[1];
shell.SelectTabCommand.Execute(survivor);
var cross = CloseButton(strip, doomed);
window.MouseDown(Centre(cross, window), MouseButton.Left);
window.MouseUp(Centre(cross, window), MouseButton.Left);
shell.Tabs.ShouldHaveSingleItem().ShouldBe(survivor);
shell.SelectedTab.ShouldBe(survivor);
});
}
[Fact]
public async Task ALeftClickOnATabSelectsItAndShowsTheTerminal()
{
await OnTheStripAsync((strip, window) =>
{
var wanted = shell.Tabs[1];
shell.ShowScreenCommand.Execute(ShellScreen.Preferences);
shell.IsTerminalShowing.ShouldBeFalse();
var button = TabButton(strip, wanted);
window.MouseDown(Centre(button, window), MouseButton.Left);
window.MouseUp(Centre(button, window), MouseButton.Left);
shell.Tabs.Count.ShouldBe(2, "selecting is not closing");
shell.SelectedTab.ShouldBe(wanted);
shell.IsTerminalShowing.ShouldBeTrue();
});
}
/// <remarks>
/// It opens the palette rather than a menu, so that the strip and Ctrl+K are one way of doing one thing.
/// See the note in <c>TerminalTabs.axaml</c> for why a flyout over the terminal's rectangle is not a
/// claim this project is willing to make without a screenshot.
/// </remarks>
[Fact]
public async Task TheButtonThatOpensAConnectionOpensThePalette()
{
await OnTheStripAsync((strip, window) =>
{
var plus = PlusButton(strip);
window.MouseDown(Centre(plus, window), MouseButton.Left);
window.MouseUp(Centre(plus, window), MouseButton.Left);
shell.IsSearching.ShouldBeTrue();
});
}
/// <remarks>
/// The strip is the one row of chrome every screen pays for, so its height is part of the layout budget
/// and this is what stops the budget drifting from the markup. See
/// <see cref="LayoutHarness.TerminalTabsHeight"/>.
/// </remarks>
[Fact]
public async Task TheStripIsTheHeightTheBudgetAssumes_AndDoesNotGrowWithTabs()
{
await LayoutHarness.OnTheUiThreadAsync(
() =>
{
for (var i = 0; i < 12; i++)
{
shell.Tabs.Add(new TerminalTabViewModel((uint)i, $"host-{i}", $"deploy@host-{i}:22"));
}
var strip = new TerminalTabs { DataContext = shell };
var window = LayoutHarness.HostAtMinimumSize(
strip, LayoutHarness.MinimumWidth, LayoutHarness.MinimumHeight);
try
{
// What it asks for, not what this host window gave it. Hosting it at 34 and then
// asserting it is 34 would pass on a strip that wanted 300 and got clipped, which is
// exactly the regression the budget needs catching.
strip.DesiredSize.Height.ShouldBe(LayoutHarness.TerminalTabsHeight);
LayoutHarness.Unreachable(window).ShouldBeEmpty();
}
finally
{
window.Close();
}
},
Token);
}
/// <summary>
/// A tab lights under the pointer, and the button that opens one is not drawn as a tab.
/// </summary>
/// <remarks>
/// <para>
/// The only test in this suite that reads a brush rather than a rectangle, and it is here because that
/// was the gap a real regression went through. Everything else measures heights and reachability, so a
/// strip whose tabs had silently stopped answering the pointer passed all of it.
/// </para>
/// <para>
/// What went wrong is worth stating, because the shape of it will recur. Avalonia has no specificity —
/// the later declaration wins — and when the tab became a pill that paints its own background, that
/// background was declared *after* the hover rule it relied on and after the exceptions the <c>+</c>
/// is made of. So every tab lost its pointer feedback and the <c>+</c> gained a fill and an outline it
/// is specifically not supposed to have. Both are one assertion each below.
/// </para>
/// </remarks>
[Fact]
public async Task ATabLightsUnderThePointer_AndThePlusIsNotDrawnAsATab()
{
await OnTheStripAsync(
(strip, window) =>
{
var tab = TabButton(strip, shell.Tabs[0]);
var resting = Fill(tab);
window.MouseMove(Centre(tab, window));
LayoutHarness.Settle(window, 900, 600);
tab.IsPointerOver.ShouldBeTrue("the pointer was moved onto it");
Fill(tab).ShouldNotBe(
resting,
"a tab that does not change under the pointer is one nobody can tell is clickable");
// Off the strip again, so the plus is measured at rest rather than under the pointer.
window.MouseMove(new Point(0, 0));
LayoutHarness.Settle(window, 900, 600);
var plus = PlusButton(strip);
Fill(plus).ShouldNotBe(
Fill(TabButton(strip, shell.Tabs[0])),
"the button that opens a connection is not one of the connections");
Presenter(plus).BorderThickness.ShouldBe(
default(Thickness),
"it carries no outline, because it is not a thing being chosen between");
});
}
// ---- Helpers ----
/// <summary>The presenter the Fluent theme actually paints, which is where every button style lands.</summary>
private static ContentPresenter Presenter(Visual button) =>
button.GetVisualDescendants()
.OfType<ContentPresenter>()
.First(presenter => presenter.Name is "PART_ContentPresenter");
/// <remarks>
/// The colour rather than the brush. Two <see cref="ISolidColorBrush"/> instances holding the same
/// colour are not equal, and it is the colour a user sees.
/// </remarks>
private static Color? Fill(Visual button) =>
Presenter(button).Background is ISolidColorBrush brush ? brush.Color : null;
/// <summary>Two open tabs, laid out in a window the width the application's is.</summary>
private Task OnTheStripAsync(Action<TerminalTabs, Window> body) =>
LayoutHarness.OnTheUiThreadAsync(
() =>
{
shell.Tabs.Add(new TerminalTabViewModel(1, "prod-db", "deploy@db.internal:22"));
shell.Tabs.Add(new TerminalTabViewModel(2, "web-01", "deploy@web-01.internal:22"));
var strip = new TerminalTabs { DataContext = shell };
var window = new Window { Content = strip };
LayoutHarness.Settle(window, 900, 600);
try
{
body(strip, window);
}
finally
{
window.Close();
}
},
Token);
/// <remarks>
/// Found by the class the style system already keys on, rather than by position in the visual tree: the
/// template puts the cross inside the tab, so both buttons carry the same data context and only the
/// classes tell them apart.
/// </remarks>
private static Button TabButton(Visual strip, TerminalTabViewModel tab) =>
strip.GetVisualDescendants()
.OfType<Button>()
.First(button => ReferenceEquals(button.DataContext, tab) && button.Classes.Contains("tab"));
/// <inheritdoc cref="TabButton" />
private static Button CloseButton(Visual strip, TerminalTabViewModel tab) =>
strip.GetVisualDescendants()
.OfType<Button>()
.First(button => ReferenceEquals(button.DataContext, tab) && button.Classes.Contains("close"));
/// <inheritdoc cref="TabButton" />
private static Button PlusButton(Visual strip) =>
strip.GetVisualDescendants().OfType<Button>().First(button => button.Classes.Contains("plus"));
private static Point Centre(Visual control, Visual window) =>
control.TranslatePoint(new Point(control.Bounds.Width / 2, control.Bounds.Height / 2), window)
?? throw new InvalidOperationException("the control is not in this window's tree");
}