Let the quick-connect palette answer for itself

Clicking outside the palette did nothing, because nothing was listening: the
wash took no pointer input at all, so the only ways out were a key and the
button that opened it. It now closes on a press whose source is the wash
itself, which is what separates outside from inside — a press on the card
bubbles through the same handler on its way to the window, and closing on
those would make the palette impossible to click into.

The caret never reached the query box either. The window focused it from the
view model's PropertyChanged, and that handler runs before the binding which
reveals the control — measured, with the same wiring, in a replica window. So
it focused a control that was still collapsed, which Avalonia treats as a
no-op and does not replay when the control is revealed, and the keyboard
stayed wherever the click that opened the palette had left it. Becoming
visible is now what triggers it, posted rather than called: a control that has
never been laid out has no visual children, and at the instant IsVisible turns
true the box still reports IsAttachedToVisualTree() == false.

Escape, Enter and the arrows move to the palette as a tunnelled handler.
Answering them only on the window was fragile in the way that matters here:
anything on the route that took a key first would silence them, and with the
focus never landing in the palette the key was being pressed at whatever the
opening click had focused — a focused Button eats Enter. The window keeps
Ctrl+K, which has to work when the palette is not showing, and forwards the
rest as the net for a press that arrives from outside the palette.

Which is also why this moved out of MainWindow rather than being fixed there.
Showing MainWindow initialises WebView2 on a thread it refuses, so nothing on
that window can be tested — the palette shipped with no test of any kind. As a
UserControl it hosts in a bare window and takes real key and pointer input,
and there are now six: press on the wash closes, press on the card does not,
Escape closes, the arrows move the selection without taking the caret out of
the box, Enter takes the highlighted host, and the palette takes the keyboard
when it appears.
This commit is contained in:
2026-07-31 10:43:30 +02:00
parent 94e11f5e38
commit 312d766c30
4 changed files with 489 additions and 64 deletions
@@ -79,9 +79,15 @@ internal sealed partial class MainWindow : Window
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// <para> /// <para>
/// A tunnelled handler rather than <c>KeyBindings</c>, because three of these four keys have to be /// Ctrl+K is here rather than on the palette because it has to work when the palette is not showing, and
/// intercepted before the control under the pointer sees them: Escape and the arrows belong to the /// it is a plain handler rather than a <c>KeyBinding</c> so that toggling stays one code path with the
/// palette while it is open, and the palette's own text box would otherwise eat them. /// rest of the chord set.
/// </para>
/// <para>
/// The palette's own keys are forwarded rather than answered: <see cref="QuickConnect"/> intercepts them
/// on their way down while the focus is inside it, and this is the net for when it is not — a press that
/// arrives with nothing focused, or from a control on the screen behind, still has to close the palette
/// rather than fall through to whatever is underneath it.
/// </para> /// </para>
/// <para> /// <para>
/// None of this reaches the terminal, and it does not need to. Once the WebView's child window holds /// None of this reaches the terminal, and it does not need to. Once the WebView's child window holds
@@ -104,61 +110,12 @@ internal sealed partial class MainWindow : Window
} }
else if (viewModel.IsSearching) else if (viewModel.IsSearching)
{ {
HandlePaletteKey(viewModel, e); Palette.HandleKey(e);
} }
base.OnKeyDown(e); base.OnKeyDown(e);
} }
/// <remarks>
/// The selection is moved here rather than by letting the list take focus, because the list taking
/// focus is exactly what would stop the query box receiving the next character typed.
/// </remarks>
private static void HandlePaletteKey(MainWindowViewModel viewModel, KeyEventArgs e)
{
switch (e.Key)
{
case Key.Escape:
viewModel.CloseSearchCommand.Execute(null);
e.Handled = true;
break;
case Key.Enter:
viewModel.ConnectToSearchResultCommand.Execute(null);
e.Handled = true;
break;
case Key.Down:
Move(viewModel, 1);
e.Handled = true;
break;
case Key.Up:
Move(viewModel, -1);
e.Handled = true;
break;
default:
break;
}
}
/// <remarks>Clamped rather than wrapped: a list that jumps from the last row to the first loses people.</remarks>
private static void Move(MainWindowViewModel viewModel, int delta)
{
if (viewModel.SearchResults.Count == 0)
{
return;
}
var current = viewModel.SelectedSearchResult is { } selected
? viewModel.SearchResults.IndexOf(selected)
: -1;
viewModel.SelectedSearchResult =
viewModel.SearchResults[Math.Clamp(current + delta, 0, viewModel.SearchResults.Count - 1)];
}
private void Attach(MainWindowViewModel? viewModel) private void Attach(MainWindowViewModel? viewModel)
{ {
if (shell is { } previous) if (shell is { } previous)
@@ -216,11 +173,18 @@ internal sealed partial class MainWindow : Window
return; return;
} }
// The palette is a text box somebody is expected to start typing into immediately, so opening it // Closing only. Opening also has to move the keyboard — the palette is a text box somebody is
// has to move the caret there — including out of the terminal, which needs the Win32 half as well. // expected to start typing into immediately — but the palette does that for itself when it becomes
// visible, which is a moment this handler is measurably ahead of: it runs from the view model's
// PropertyChanged, before the binding that reveals the control, and Focus() on a control that is
// still collapsed is a no-op that is not replayed when it is revealed.
if (string.Equals(e.PropertyName, nameof(MainWindowViewModel.IsSearching), StringComparison.Ordinal)) if (string.Equals(e.PropertyName, nameof(MainWindowViewModel.IsSearching), StringComparison.Ordinal))
{ {
ReleaseKeyboardTo(viewModel.IsSearching ? Palette.QueryBox : KeyboardHome); if (!viewModel.IsSearching)
{
ReleaseKeyboardTo(KeyboardHome);
}
return; return;
} }
@@ -21,7 +21,13 @@
an overlay across the middle of this window would be painted underneath it and take no clicks. an overlay across the middle of this window would be painted underneath it and take no clicks.
--> -->
<Border Background="#CC0A0C0B"> <!--
The wash is named and takes a press, because clicking away from a palette is how every palette closes.
The handler answers only for presses whose source is the wash itself, which is what separates "outside"
from "inside": a press anywhere on the card below reports that control as its source and bubbles through
here on its way to the window.
-->
<Border x:Name="Backdrop" Background="#CC0A0C0B" PointerPressed="OnBackdropPressed">
<Border Width="520" VerticalAlignment="Top" Margin="0,90,0,0" <Border Width="520" VerticalAlignment="Top" Margin="0,90,0,0"
Background="{StaticResource Chrome}" BorderBrush="{StaticResource BorderMid}" Background="{StaticResource Chrome}" BorderBrush="{StaticResource BorderMid}"
BorderThickness="1" CornerRadius="6"> BorderThickness="1" CornerRadius="6">
@@ -39,8 +45,8 @@
</Border> </Border>
<!-- <!--
Arrow keys move the selection and Enter takes it; the window's key handler owns both, because a Arrow keys move the selection and Enter takes it; the code-behind owns both, because a ListBox that
ListBox that took focus would take the arrow keys away from the box being typed into. took focus would take the arrow keys away from the box being typed into.
--> -->
<ListBox x:Name="Results" MaxHeight="280" Focusable="False" <ListBox x:Name="Results" MaxHeight="280" Focusable="False"
ItemsSource="{Binding SearchResults}" ItemsSource="{Binding SearchResults}"
@@ -1,17 +1,174 @@
using Avalonia;
using Avalonia.Controls; using Avalonia.Controls;
using Avalonia.Input;
using Avalonia.Interactivity;
using Avalonia.Threading;
using DodoSSH.Client.App.ViewModels;
namespace DodoSSH.Client.App.Views; namespace DodoSSH.Client.App.Views;
/// <summary>The Ctrl+K host search, overlaid on the window.</summary> /// <summary>The Ctrl+K host search, overlaid on the window.</summary>
/// <remarks> /// <remarks>
/// The keys it responds to are handled by <see cref="MainWindow"/> rather than here, because two of them — /// <para>
/// Ctrl+K to open and Escape to close — have to work when this control does not exist yet or has just /// Everything a palette does once it is open belongs here rather than in <see cref="MainWindow"/>: the four
/// stopped existing. Handling the arrows in the same place keeps the whole chord set in one method. /// keys it answers to, the press outside it that dismisses it, and taking the keyboard the moment it appears.
/// Opening it is the window's business, because Ctrl+K has to work when this control is not showing.
/// </para>
/// <para>
/// The split used to fall the other way, and the cost was that none of it could be tested. Showing
/// <see cref="MainWindow"/> initialises WebView2, which refuses the headless dispatcher's thread — see
/// <c>LayoutHarnessTests.WhyTheWindowItselfIsNeverShown</c> — so behaviour that lived on the window could only
/// be checked by hand. A <see cref="UserControl"/> hosts in a bare window and takes real key and pointer
/// input.
/// </para>
/// </remarks> /// </remarks>
internal sealed partial class QuickConnect : UserControl internal sealed partial class QuickConnect : UserControl
{ {
public QuickConnect() => InitializeComponent(); public QuickConnect()
{
InitializeComponent();
/// <summary>The box, so the window can put the caret in it the moment the palette opens.</summary> // Tunnelled, and deliberately: the query box below is on the route these keys take, and a text box
// that grows a use for Enter or the arrows — a multi-line box, a completion list — would take them
// before a bubbling handler here ever ran. The palette owns them while it is open, so it says so at
// the point on the route where nothing else has had a chance yet.
AddHandler(KeyDownEvent, OnPaletteKey, RoutingStrategies.Tunnel);
}
/// <summary>The box, so the caret can be put in it the moment the palette opens.</summary>
internal TextBox QueryBox => Query; internal TextBox QueryBox => Query;
private MainWindowViewModel? Shell => DataContext as MainWindowViewModel;
/// <summary>
/// Answers one of the palette's keys, wherever in the window it was pressed.
/// </summary>
/// <remarks>
/// Internal because <see cref="MainWindow"/> calls it too, for the case this control's own tunnelled
/// handler cannot see: a key routes through here only while the focus is inside the palette, and the
/// window is what catches Escape when it is not.
/// </remarks>
internal void HandleKey(KeyEventArgs e)
{
ArgumentNullException.ThrowIfNull(e);
if (Shell is not { IsSearching: true } shell)
{
return;
}
switch (e.Key)
{
case Key.Escape:
shell.CloseSearchCommand.Execute(null);
e.Handled = true;
break;
case Key.Enter:
shell.ConnectToSearchResultCommand.Execute(null);
e.Handled = true;
break;
case Key.Down:
Move(shell, 1);
e.Handled = true;
break;
case Key.Up:
Move(shell, -1);
e.Handled = true;
break;
default:
break;
}
}
/// <summary>
/// Takes the keyboard, so the palette can be typed into the instant it appears.
/// </summary>
/// <remarks>
/// <para>
/// Both halves, for the reason <see cref="NativeKeyboardFocus"/> gives: the terminal's WebView is a native
/// child window that keeps Win32 focus even after it is collapsed, so focusing an Avalonia control without
/// the Win32 call produces a box with a caret in it that silently receives nothing.
/// </para>
/// <para>
/// Done here rather than by the window, and that is the fix rather than a tidying. <c>Focus()</c> on a
/// collapsed control is measurably a no-op that is not replayed when the control is revealed, and the
/// window's own attempt ran from the view model's <c>PropertyChanged</c> — ahead of the binding that makes
/// this control visible, so it focused a control that was still collapsed and the keyboard stayed wherever
/// it was.
/// </para>
/// <para>
/// Becoming visible is still too early on its own, which is why this is posted rather than called. A
/// control that has never been laid out has no visual children — measured: at the instant
/// <c>IsVisible</c> turns true the query box reports <c>IsAttachedToVisualTree() == false</c>, and focus is
/// refused to anything not in the tree. Layout runs at a higher priority than this callback, so by the
/// time it is picked up the box exists.
/// </para>
/// </remarks>
private void TakeKeyboard()
{
// The palette can have been dismissed between the post and the callback — a press on the wash, or a
// second Ctrl+K — and stealing the keyboard back into a control nobody can see would be worse than
// arriving late.
if (!IsVisible)
{
return;
}
if (TopLevel.GetTopLevel(this) is Window window)
{
NativeKeyboardFocus.ReturnTo(window);
}
Query.Focus();
}
/// <remarks>Clamped rather than wrapped: a list that jumps from the last row to the first loses people.</remarks>
private static void Move(MainWindowViewModel shell, int delta)
{
if (shell.SearchResults.Count == 0)
{
return;
}
var current = shell.SelectedSearchResult is { } selected
? shell.SearchResults.IndexOf(selected)
: -1;
shell.SelectedSearchResult =
shell.SearchResults[Math.Clamp(current + delta, 0, shell.SearchResults.Count - 1)];
}
/// <inheritdoc />
protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change)
{
// Base first, so the visibility this control has just been given has already reached its descendants:
// focus is refused to anything not effectively visible, and the box below is a descendant.
base.OnPropertyChanged(change);
if (change.Property == IsVisibleProperty && change.GetNewValue<bool>())
{
Dispatcher.UIThread.Post(TakeKeyboard, DispatcherPriority.Loaded);
}
}
private void OnPaletteKey(object? sender, KeyEventArgs e) => HandleKey(e);
/// <remarks>
/// Only a press on the wash itself. Presses on the card bubble through here as well, and closing on those
/// would make the palette impossible to click into.
/// </remarks>
private void OnBackdropPressed(object? sender, PointerPressedEventArgs e)
{
if (!ReferenceEquals(e.Source, Backdrop) || Shell is not { IsSearching: true } shell)
{
return;
}
shell.CloseSearchCommand.Execute(null);
e.Handled = true;
}
} }
@@ -0,0 +1,298 @@
using Avalonia;
using Avalonia.Controls;
using Avalonia.Headless;
using Avalonia.Input;
using Avalonia.Threading;
using Avalonia.VisualTree;
using DodoSSH.Client.App.ViewModels;
using DodoSSH.Client.App.Views;
using DodoSSH.Client.Session;
using DodoSSH.Client.Session.Tests;
using DodoSSH.Client.Ssh;
using DodoSSH.Client.Storage;
using DodoSSH.Client.Terminal;
using DodoSSH.Crypto;
using NSubstitute;
namespace DodoSSH.Client.App.Layout.Tests;
/// <summary>
/// How the quick-connect palette answers a keyboard and a pointer.
/// </summary>
/// <remarks>
/// <para>
/// This is the suite the palette shipped without, and the reason it shipped without one is that all of this
/// used to live on <c>MainWindow</c> — which cannot be shown here at all, because attaching the terminal's
/// WebView initialises WebView2 on a thread it refuses. See
/// <see cref="LayoutHarnessTests.WhyTheWindowItselfIsNeverShown"/>. A <c>UserControl</c> hosts in a bare
/// window, takes real key and pointer input, and can therefore be held to what it promises.
/// </para>
/// <para>
/// Three things were wrong and each has a test here: nothing answered a press outside the palette, so the one
/// gesture everybody tries first did nothing; the caret never reached the query box, because the window
/// focused it from the view model's <c>PropertyChanged</c> — ahead of the binding that reveals the control,
/// and focus on a collapsed control is a no-op; and the keys were answered only by a handler on the window,
/// which anything on the route could have taken first.
/// </para>
/// <para>
/// A real <see cref="MainWindowViewModel"/> over a real unlocked vault, for the same reason the layout suite
/// uses one: compiled bindings resolve against the declared type, and the palette's list is populated by the
/// vault's own hosts. Nothing here reaches a network — the connect the Enter test performs fails inside the
/// vault's own error handling, which is fine, because what Enter promises is to take the highlighted result
/// and close.
/// </para>
/// </remarks>
public sealed class QuickConnectTests : IAsyncLifetime
{
private const string Passphrase = "a sufficiently long passphrase";
private const string ServerUrl = "https://dodossh.example";
/// <remarks>Far below the shipped profile: nothing here attacks a wrap.</remarks>
private static readonly Argon2Profile CheapProfile =
Argon2Profile.FromStoredParameters(memoryKibibytes: 8 * 1024, passes: 1, parallelism: 1);
private readonly FakeAccountServer server = new();
private readonly StubKeyBinding keyBinding = new();
private readonly VaultKnownHostStore knownHosts = new();
private ClientCacheFactory caches = null!;
private TerminalWorkspace workspace = null!;
private VaultSession session = null!;
private VaultViewModel vault = null!;
private MainWindowViewModel shell = null!;
private static CancellationToken Token => TestContext.Current.CancellationToken;
/// <inheritdoc />
public async ValueTask InitializeAsync()
{
caches = ClientCacheFactory.ForMemory($"palette-{Guid.CreateVersion7():N}");
await caches.MigrateAsync(Token);
await new AccountProvisioner(server, keyBinding, caches, TimeProvider.System, CheapProfile)
.EnrollAsync(ServerUrl, Passphrase, "laptop", "Personal", Token);
var outcome = await new SessionOpener(caches, TimeProvider.System).UnlockAsync(Passphrase, Token);
outcome.IsUnlocked.ShouldBeTrue(outcome.Message);
session = outcome.Session!;
workspace = new TerminalWorkspace(
new InMemoryTerminalAssetProvider(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)),
Substitute.For<ISshConnectionFactory>(),
TimeProvider.System);
await knownHosts.OpenAsync(session, Token);
vault = new VaultViewModel(session, workspace, knownHosts, static () => null);
await SeedAsync();
shell = new MainWindowViewModel(
ClientPaths.Default,
caches,
workspace,
knownHosts,
Substitute.For<IDeviceKeyStore>(),
(_, _) => throw new NotSupportedException("nothing here signs in"),
TimeProvider.System,
CheapProfile)
{
// The state the palette is only ever open in. Assigned rather than reached through the unlock
// path, which would be a second enrollment and a second Argon2 pass for no extra coverage.
State = ShellState.Unlocked,
Vault = vault,
};
}
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
await shell.DisposeAsync();
knownHosts.Close();
await workspace.DisposeAsync();
await session.DisposeAsync();
caches.Dispose();
}
/// <remarks>
/// The gesture everybody tries first, and the one that did nothing at all: the wash took no pointer input,
/// so the only ways out of the palette were a key and the button that opened it.
/// </remarks>
[Fact]
public async Task APressOnTheWashClosesThePalette()
{
await OnThePaletteAsync((_, window) =>
{
// The bottom-left corner: the card is 520 wide, centred, and starts 90 pixels down, so nothing
// here belongs to it.
window.MouseDown(new Point(12, 520), MouseButton.Left);
shell.IsSearching.ShouldBeFalse();
});
}
/// <remarks>
/// The other half of the same rule, and the one that makes it worth a handler rather than a press anywhere
/// closing: a press on the card bubbles through the wash on its way out, so a handler that did not check
/// where the press started would close the palette the moment somebody clicked into the box.
/// </remarks>
[Fact]
public async Task APressOnTheCardDoesNotClose()
{
await OnThePaletteAsync((palette, window) =>
{
window.MouseDown(Centre(palette.QueryBox, window), MouseButton.Left);
shell.IsSearching.ShouldBeTrue();
});
}
[Fact]
public async Task EscapeClosesThePalette()
{
await OnThePaletteAsync((palette, window) =>
{
palette.QueryBox.Focus().ShouldBeTrue();
window.KeyPressQwerty(PhysicalKey.Escape, RawInputModifiers.None);
shell.IsSearching.ShouldBeFalse();
});
}
/// <remarks>
/// The second assertion is the whole reason the selection is moved by hand rather than by letting the list
/// take focus: a palette whose arrow keys moved the caret out of the query box would stop receiving the
/// next character typed.
/// </remarks>
[Fact]
public async Task TheArrowsMoveTheSelectionAndLeaveTheKeyboardInTheBox()
{
await OnThePaletteAsync((palette, window) =>
{
palette.QueryBox.Focus().ShouldBeTrue();
shell.SearchResults.Count.ShouldBeGreaterThan(2, "an empty list would prove nothing");
shell.SelectedSearchResult.ShouldBe(shell.SearchResults[0]);
window.KeyPressQwerty(PhysicalKey.ArrowDown, RawInputModifiers.None);
shell.SelectedSearchResult.ShouldBe(shell.SearchResults[1]);
window.KeyPressQwerty(PhysicalKey.ArrowUp, RawInputModifiers.None);
shell.SelectedSearchResult.ShouldBe(shell.SearchResults[0]);
// Clamped rather than wrapped, which is the palette's own rule.
window.KeyPressQwerty(PhysicalKey.ArrowUp, RawInputModifiers.None);
shell.SelectedSearchResult.ShouldBe(shell.SearchResults[0]);
palette.QueryBox.IsFocused.ShouldBeTrue("the arrows must not move the caret out of the box");
});
}
/// <remarks>
/// What Enter promises is to take the highlighted row: the palette closes and the vault is pointed at that
/// host. The connection it then asks for fails in this suite — there is no server and no shell — and it
/// fails inside the vault's own handling, which is the point of connecting through the vault's command
/// rather than opening a session from the palette.
/// </remarks>
[Fact]
public async Task EnterTakesTheHighlightedResult()
{
await OnThePaletteAsync((palette, window) =>
{
palette.QueryBox.Focus().ShouldBeTrue();
window.KeyPressQwerty(PhysicalKey.ArrowDown, RawInputModifiers.None);
var highlighted = shell.SelectedSearchResult.ShouldNotBeNull();
window.KeyPressQwerty(PhysicalKey.Enter, RawInputModifiers.None);
shell.IsSearching.ShouldBeFalse();
vault.SelectedHost?.EntityId.ShouldBe(highlighted.EntityId);
});
}
/// <remarks>
/// The palette is a box somebody is expected to start typing into, and for a while it was not: the window
/// focused it from the view model's <c>PropertyChanged</c>, which runs before the binding that reveals the
/// control, and <c>Focus()</c> on a collapsed control is a no-op that is never replayed. Becoming visible
/// is the moment that cannot be too early, so that is where the palette takes the keyboard — and this is
/// the test that says so.
/// </remarks>
[Fact]
public async Task ThePaletteTakesTheKeyboardWhenItAppears()
{
await LayoutHarness.OnTheUiThreadAsync(
() =>
{
var elsewhere = new TextBox();
var palette = new QuickConnect { DataContext = shell, IsVisible = false };
var window = new Window { Content = new Panel { Children = { elsewhere, palette } } };
LayoutHarness.Settle(window, 900, 600);
try
{
elsewhere.Focus().ShouldBeTrue();
shell.ToggleSearchCommand.Execute(null);
palette.IsVisible = true;
// The layout pass the application's dispatcher would run anyway. Without it the query box
// is not in the visual tree yet, which is the whole reason the palette defers this.
Dispatcher.UIThread.RunJobs();
palette.QueryBox.IsFocused.ShouldBeTrue();
}
finally
{
window.Close();
}
},
Token);
}
// ---- Helpers ----
/// <summary>Opens the palette in a window the size the application's is, and runs one body against it.</summary>
private Task OnThePaletteAsync(Action<QuickConnect, Window> body) =>
LayoutHarness.OnTheUiThreadAsync(
() =>
{
shell.ToggleSearchCommand.Execute(null);
shell.IsSearching.ShouldBeTrue("every case here starts with the palette open");
var palette = new QuickConnect { DataContext = shell };
var window = new Window { Content = palette };
LayoutHarness.Settle(window, 900, 600);
try
{
body(palette, window);
}
finally
{
window.Close();
}
},
Token);
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");
/// <remarks>Enough hosts that the arrow keys have somewhere to go.</remarks>
private async Task SeedAsync()
{
for (var i = 0; i < 6; i++)
{
vault.NewHostCommand.Execute(null);
vault.EditorLabel = $"host-{i}";
vault.EditorHostname = $"host-{i}.internal";
vault.EditorUsername = "deploy";
await vault.SaveHostCommand.ExecuteAsync(null);
}
await vault.LoadAsync(Token);
}
}