From 36b8a23020207af8031e887211020e0be2b7f554 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Thu, 6 Aug 2026 12:35:06 +0200 Subject: [PATCH] Give the update banner the view model it is typed to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/DodoSSH.Client.App/Views/MainWindow.axaml | 15 ++- .../UpdateBannerTests.cs | 94 +++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml b/src/DodoSSH.Client.App/Views/MainWindow.axaml index e58d9cc..0fdc74b 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml @@ -360,12 +360,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. --> + DataContext="{Binding Updates}" + IsVisible="{Binding IsBannerShowing, FallbackValue=False}" /> diff --git a/tests/DodoSSH.Client.App.Layout.Tests/UpdateBannerTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/UpdateBannerTests.cs index 8d5be07..c051f17 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/UpdateBannerTests.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/UpdateBannerTests.cs @@ -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); } + + /// + /// The banner in the window that shows it, rather than in a host window this suite built. + /// + /// + /// + /// The two tests above hand the control an 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 MainWindow 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 Commands null, and hovered and + /// depressed like a working banner while neither button did anything. + /// + /// + /// 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. + /// + /// + /// The window is constructed and never shown, for the reason + /// 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. + /// + /// + [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(StringComparer.Ordinal)), + Substitute.For(), + 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(), + (_, _) => throw new NotSupportedException("nothing here signs in"), + TimeProvider.System, + Substitute.For(), + 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().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