Public Access
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
This commit is contained in:
@@ -318,10 +318,38 @@
|
||||
<Setter Property="Foreground" Value="{StaticResource TextDim}" />
|
||||
</Style>
|
||||
|
||||
<!--
|
||||
◆ ORDER IS THE BEHAVIOUR HERE. The base pill comes first, then its hover, then the exceptions — and
|
||||
that ordering is the fix for a bug this arrangement had when v2 first drew it.
|
||||
|
||||
A pill paints its own Background, which the flat tab it replaced did not. That one detail moves where
|
||||
the hover has to live: `Button.flat:pointerover` is declared far above and used to supply it, and the
|
||||
moment `.tab`'s template rule set a Background of its own, the later declaration won and every tab in
|
||||
the strip lost its pointer feedback silently. The `+` lost more than that — it sits below as an
|
||||
exception, so a `.tab` rule declared after it was overriding the very thing that made it an exception,
|
||||
and it drew as a filled outlined pill contradicting the comment above it.
|
||||
|
||||
Avalonia has no specificity; the later declaration wins. The same trap is recorded further down this
|
||||
file for Border.rowmark. Anything added below must be an exception to what is above it, never a
|
||||
restatement of the base.
|
||||
-->
|
||||
<Style Selector="Button.tab /template/ ContentPresenter#PART_ContentPresenter">
|
||||
<Setter Property="Foreground" Value="{StaticResource TextDim}" />
|
||||
<Setter Property="Background" Value="{StaticResource Panel}" />
|
||||
<Setter Property="BorderBrush" Value="{StaticResource Border}" />
|
||||
<Setter Property="BorderThickness" Value="1" />
|
||||
<Setter Property="CornerRadius" Value="8" />
|
||||
</Style>
|
||||
<Style Selector="Button.tab:pointerover /template/ ContentPresenter#PART_ContentPresenter">
|
||||
<Setter Property="Background" Value="{StaticResource Hover}" />
|
||||
<Setter Property="Foreground" Value="{StaticResource Text}" />
|
||||
</Style>
|
||||
|
||||
<!--
|
||||
The button that opens a connection. A tab in every respect but the marks a tab carries: no active
|
||||
state, because it is never the thing showing, and no outline, because it is not one of the things
|
||||
being chosen between.
|
||||
being chosen between. Both of those are cleared rather than merely omitted — the base rule above sets
|
||||
a fill and a border, so an exception has to say so.
|
||||
-->
|
||||
<Style Selector="Button.tab.plus">
|
||||
<Setter Property="Padding" Value="0" />
|
||||
@@ -330,6 +358,7 @@
|
||||
<Setter Property="Foreground" Value="{StaticResource TextFaint}" />
|
||||
</Style>
|
||||
<Style Selector="Button.tab.plus /template/ ContentPresenter#PART_ContentPresenter">
|
||||
<Setter Property="Background" Value="Transparent" />
|
||||
<Setter Property="BorderThickness" Value="0" />
|
||||
<Setter Property="CornerRadius" Value="8" />
|
||||
</Style>
|
||||
@@ -337,13 +366,7 @@
|
||||
<Setter Property="Foreground" Value="{StaticResource Text}" />
|
||||
<Setter Property="Background" Value="{StaticResource Hover}" />
|
||||
</Style>
|
||||
<Style Selector="Button.tab /template/ ContentPresenter#PART_ContentPresenter">
|
||||
<Setter Property="Foreground" Value="{StaticResource TextDim}" />
|
||||
<Setter Property="Background" Value="{StaticResource Panel}" />
|
||||
<Setter Property="BorderBrush" Value="{StaticResource Border}" />
|
||||
<Setter Property="BorderThickness" Value="1" />
|
||||
<Setter Property="CornerRadius" Value="8" />
|
||||
</Style>
|
||||
|
||||
<Style Selector="Button.tab.active /template/ ContentPresenter#PART_ContentPresenter">
|
||||
<Setter Property="Background" Value="{StaticResource Active}" />
|
||||
<Setter Property="Foreground" Value="{StaticResource Text}" />
|
||||
|
||||
@@ -58,11 +58,19 @@
|
||||
command", and there is no snippet or saved-command item type for it to run. Clicking it is the same
|
||||
as Ctrl+K, which is what the window binds; the design says ⌘K, and this is a Windows build.
|
||||
-->
|
||||
<!--
|
||||
Stretch-to-a-maximum, not a fixed width. The design draws this box at exactly 380 and centred, and
|
||||
stating that as a Width on the Border is what makes 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 the stretching button gives the same 380 whenever there is room and gives
|
||||
way when there is not.
|
||||
-->
|
||||
<Button Grid.Column="1" Classes="flat" MaxWidth="380" Height="28" Margin="16,0"
|
||||
HorizontalAlignment="Center" HorizontalContentAlignment="Stretch"
|
||||
HorizontalAlignment="Stretch" HorizontalContentAlignment="Stretch"
|
||||
Command="{Binding ToggleSearchCommand}" IsEnabled="{Binding IsUnlocked}">
|
||||
<Border Background="{StaticResource Field}" BorderBrush="{StaticResource BorderMid}"
|
||||
BorderThickness="1" CornerRadius="8" Padding="10,0" Width="380">
|
||||
BorderThickness="1" CornerRadius="8" Padding="10,0">
|
||||
<Grid ColumnDefinitions="Auto,*,Auto">
|
||||
<TextBlock Grid.Column="0" Text="⌕" FontSize="12"
|
||||
Foreground="{StaticResource TextFaint}" VerticalAlignment="Center" />
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
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;
|
||||
@@ -241,8 +243,72 @@ public sealed class TerminalTabsTests : IAsyncLifetime
|
||||
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(
|
||||
|
||||
Reference in New Issue
Block a user