2 Commits
Author SHA1 Message Date
jaap-jan 7b616e0bb0 Merge branch 'claude/windows-update-bar-buttons-b19b2d'
ci / build and test (push) Successful in 1m55s
ci / android head (push) Successful in 3m25s
ci / desktop nightly (push) Successful in 43s
ci / api image (push) Successful in 24s
2026-08-06 12:36:18 +02:00
jaap-jan 36b8a23020 Give the update banner the view model it is typed to
The banner has never worked. It went into MainWindow's fourth row with no data
context of its own, so it inherited the shell's — and it is the one control in
that file typed to a screen's view model rather than to MainWindowViewModel,
because it is the only one with a layout suite that hosts it over UpdateViewModel
alone. Compiled bindings type-check against x:DataType at runtime, so every
binding inside it resolved against the wrong object and failed the way a compiled
binding does: quietly. No headline, and DismissBannerCommand and RestartNowCommand
both null.

A button with a null command is enabled, hovers, depresses and does nothing, which
is why this looked like a hit-testing problem and why the WebView was the first
suspect. It is not one. The strip is a sibling row for the reason the occlusion
rule gives and that arrangement is correct — the terminal's rectangle is never
covered, only shortened. What was actually on offer was an announcement that an
update had been downloaded, with two buttons that refused to install it and no
way to make it go away either. The preferences screen's RESTART NOW worked
throughout, because it binds Updates.RestartNowCommand from the shell's own
context, which is the contrast that pins the cause.

The context is set on the banner itself and IsVisible loses its Updates. prefix
with it, because a data context on an element resolves that element's other
bindings too — the rule the page area's wrappers upstairs exist to work around.
Those wrappers are needed because IsHostsScreen and its siblings belong to the
shell; IsBannerShowing belongs to the banner's own view model, so there is nothing
to wrap here.

Neither existing suite could have caught it. A layout test supplies the data
context it is measuring, which is exactly the assumption that was wrong, and the
shell suite has no visual tree — its project file already says it does not cover
whether the XAML binds to the right names. So the new test asserts the wiring
rather than the layout: a real shell over the ready-update fake, MainWindow
constructed and never shown, and the banner asked what context it got, whether it
is visible and whether RESTART NOW carries a command. Checked failing with the one
attribute removed. Constructing the window is safe where showing it is not, and
nothing here needs it shown: a data context propagates when it is set, not when
the tree is measured.
2026-08-06 12:35:06 +02:00
2 changed files with 108 additions and 1 deletions
+14 -1
View File
@@ -376,12 +376,25 @@
buttons and a version string of unknown length is exactly the shape that arranges one of them off the
edge. See UpdateBanner.axaml.
◆ THE DATA CONTEXT IS SET HERE, and the banner did nothing at all until it was.
Unlike the titlebar and the status bar, which are typed to this window's own view model and inherit its
context, the banner is typed to UpdateViewModel — it is one screen's control and its layout suite hosts
it over that view model alone. Inheriting the shell's context instead left every compiled binding
inside it resolving against the wrong type and failing silently: no headline, and both Commands null,
so the strip appeared, hovered and pressed like a real banner and neither button did anything.
IsVisible is unqualified because the context is set on this same element, which resolves it against
UpdateViewModel too — the rule the page area's wrappers above are wrapped for. It needs no wrapper: the
flag it binds is the banner's own, unlike IsHostsScreen and its siblings, which belong to the shell.
FallbackValue, for the reason the WebView and the connecting card carry one: a compiled binding with
no DataContext yields UnsetValue, IsVisible falls back to true, and the previewer would show a banner
announcing an update that does not exist.
-->
<views:UpdateBanner Grid.Row="2"
IsVisible="{Binding Updates.IsBannerShowing, FallbackValue=False}" />
DataContext="{Binding Updates}"
IsVisible="{Binding IsBannerShowing, FallbackValue=False}" />
<views:StatusBar Grid.Row="3" />
@@ -1,7 +1,12 @@
using Avalonia.Controls;
using Avalonia.LogicalTree;
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;
@@ -145,4 +150,93 @@ public sealed class UpdateBannerTests
},
Token);
}
/// <summary>
/// The banner in the window that shows it, rather than in a host window this suite built.
/// </summary>
/// <remarks>
/// <para>
/// The two tests above hand the control an <see cref="UpdateViewModel"/> themselves — which is precisely
/// what the window did not do. The banner is the one control here typed to a screen's view model instead
/// of to the shell's, and it was dropped into <c>MainWindow</c> with no data context at all, so it
/// inherited the shell's: every compiled binding inside it then resolved against the wrong type and
/// failed silently. The strip appeared, with no headline and both <c>Command</c>s null, and hovered and
/// depressed like a working banner while neither button did anything.
/// </para>
/// <para>
/// Nothing else could have caught it. A layout test supplies the context it is measuring, and the shell
/// suite has no visual tree — its own project file says as much: it does not cover whether the XAML binds
/// to the right names.
/// </para>
/// <para>
/// The window is constructed and never shown, for the reason
/// <see cref="LayoutHarnessTests.WhyTheWindowItselfIsNeverShown"/> gives, and it does not need to be: a
/// data context propagates and a binding resolves when the context is set, not when the tree is measured.
/// So this asserts the wiring and leaves every question of size to the two tests above.
/// </para>
/// </remarks>
[Fact]
public async Task TheWindowHandsTheBannerTheUpdateViewModel()
{
var directory = Path.Combine(Path.GetTempPath(), $"dodossh-banner-shell-{Guid.CreateVersion7():N}");
using var caches = ClientCacheFactory.ForMemory($"banner-{Guid.CreateVersion7():N}");
await using var workspace = new TerminalWorkspace(
new InMemoryTerminalAssetProvider(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)),
Substitute.For<ISshConnectionFactory>(),
TimeProvider.System);
// The real shell view model, because the thing under test is what the window hands the banner. No
// vault and no sign-in: the banner is drawn outside the unlocked half of the window on purpose.
await using var shell = new MainWindowViewModel(
new ClientPaths(directory),
caches,
workspace,
new VaultKnownHostStore(),
Substitute.For<IDeviceKeyStore>(),
(_, _) => throw new NotSupportedException("nothing here signs in"),
TimeProvider.System,
Substitute.For<ISftpSessionFactory>(),
updates: new ReadyChannel());
try
{
await shell.Updates.CheckOnceAsync(Token);
shell.Updates.IsBannerShowing.ShouldBeTrue();
await LayoutHarness.OnTheUiThreadAsync(() => AssertTheBannerIsWiredTo(shell), Token);
}
finally
{
if (Directory.Exists(directory))
{
Directory.Delete(directory, recursive: true);
}
}
}
private static void AssertTheBannerIsWiredTo(MainWindowViewModel shell)
{
var window = new MainWindow { DataContext = shell };
try
{
var banner = window.GetLogicalDescendants().OfType<UpdateBanner>().ShouldHaveSingleItem();
banner.DataContext.ShouldBeSameAs(shell.Updates);
banner.IsVisible.ShouldBeTrue();
// The defect itself: a null command is a button that answers a click by doing nothing, and it
// is indistinguishable from a working one until somebody presses it.
var restart = banner.FindControl<Button>("RestartNowButton").ShouldNotBeNull();
restart.Command.ShouldNotBeNull();
}
finally
{
window.Close();
}
}
}