From d6d576c85face84775729349a67052f10496cf72 Mon Sep 17 00:00:00 2001 From: James Maes Date: Sun, 5 Jul 2026 07:52:32 -0500 Subject: [PATCH 1/3] Harden account-org display against WinUI editable-ComboBox blanking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating "saved account loads with a blank org dropdown": the account store, vault entries, and view-model seeding are all intact (a live probe of a seeded account shows the inner Edit populated), but the symptom matches two known editable-ComboBox behaviors, both now guarded: - the control never renders Text bound before its template loads; all three org combos (connect card, chip flyout, wizard step 3) now re-apply the value into the template TextBox on Loaded (EditableComboBox.ReapplyText — re-setting Text alone is an identical-value no-op) - mutating ItemsSource resets the control's Text; the org-list refresh now captures, restores, and re-raises Organization so a seeded or typed value survives (regression test) Co-Authored-By: Claude Fable 5 --- gclo.Engine.Tests/WorkspaceViewModelTests.cs | 36 ++++++++++++ gclo.ViewModels/WorkspaceViewModel.cs | 8 +++ gclo/AccountWizardDialog.xaml | 1 + gclo/AccountWizardDialog.xaml.cs | 6 ++ gclo/EditableComboBox.cs | 60 ++++++++++++++++++++ gclo/WorkspacePage.xaml | 2 + gclo/WorkspacePage.xaml.cs | 7 +++ 7 files changed, 120 insertions(+) create mode 100644 gclo/EditableComboBox.cs diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index 0379e2b..516bb92 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -1150,6 +1150,42 @@ public sealed class WorkspaceViewModelTests MaxConcurrency = 1, // serialize progress: the sync fake reports inline across threads }; + [Fact] + public async Task OrgRefresh_AccountSeededOrganization_SurvivesAndResyncsTheView() + { + // Regression: the view's editable ComboBox resets its Text when its + // ItemsSource is mutated, blanking the org an account workspace seeded in + // the constructor. The VM must keep the value across the refresh AND raise + // Organization again after the list mutation so the view resyncs its Text. + var account = new Account + { + Id = Guid.NewGuid(), + Name = "Kof", + Organization = "KofTwentyTwo", + TargetRoot = @"C:\repos\kof", + }; + var vault = new InMemoryVault(); + vault.Store(account.Id, "vault-token-1234567890"); + _orgs.Handler = (_, _) => + Task.FromResult>(["KofTwentyTwo", "acme", "other"]); + + var vm = CreateViewModel(account: account, vault: vault); + bool resyncAfterListArrived = false; + vm.PropertyChanged += (_, e) => + { + if (e.PropertyName == nameof(WorkspaceViewModel.Organization) + && vm.Organizations.Count > 0) + { + resyncAfterListArrived = true; + } + }; + + await WaitUntilAsync(() => vm.Organizations.Count == 3, "org list refresh"); + + Assert.Equal("KofTwentyTwo", vm.Organization); + Assert.True(resyncAfterListArrived, "Organization must be re-raised after the list mutation"); + } + [Fact] public async Task AccountConstruction_SeedsWorkspace_AndLooksUpOrgsWithVaultToken() { diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index fff3534..8e5ad61 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -352,11 +352,19 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable var orgs = await _orgLister.ListOrganizationsAsync(token, lookupToken); lookupToken.ThrowIfCancellationRequested(); + // An editable ComboBox resets its Text when its ItemsSource is mutated, + // and the TwoWay binding would wipe a value that was already set — an + // account workspace seeds Organization in the constructor, and this + // refresh lands ~a second later. Capture, restore, and force a binding + // resync so the seeded (or typed) organization survives the refresh. + string organizationBeforeRefresh = Organization; Organizations.Clear(); foreach (string org in orgs) { Organizations.Add(org); } + Organization = organizationBeforeRefresh; + OnPropertyChanged(nameof(Organization)); // The production lister always lists the token's own account first, so a // single entry means no organizations were visible. StatusText = orgs.Count <= 1 diff --git a/gclo/AccountWizardDialog.xaml b/gclo/AccountWizardDialog.xaml index f605ac8..5b98026 100644 --- a/gclo/AccountWizardDialog.xaml +++ b/gclo/AccountWizardDialog.xaml @@ -102,6 +102,7 @@ IsEditable="True" ItemsSource="{x:Bind ViewModel.Organizations, Mode=OneTime}" PlaceholderText="Choose from the list, or type a name" + Loaded="OrgBox_Loaded" Text="{x:Bind ViewModel.Organization, Mode=TwoWay}" /> diff --git a/gclo/AccountWizardDialog.xaml.cs b/gclo/AccountWizardDialog.xaml.cs index 7929b94..363697e 100644 --- a/gclo/AccountWizardDialog.xaml.cs +++ b/gclo/AccountWizardDialog.xaml.cs @@ -149,6 +149,12 @@ namespace gclo private void TokenBox_PasswordChanged(object sender, RoutedEventArgs e) => ViewModel.Token = ((PasswordBox)sender).Password; + // An editable ComboBox does not render Text set before its template loaded; + // when editing an account, step 3 would show blank despite the seeded + // organization — see EditableComboBox. + private void OrgBox_Loaded(object sender, RoutedEventArgs e) + => EditableComboBox.ReapplyText((ComboBox)sender, ViewModel.Organization); + /// Mirrors the NumberBox's double into the view model's int. private void ConcurrencyBox_ValueChanged(NumberBox sender, NumberBoxValueChangedEventArgs args) { diff --git a/gclo/EditableComboBox.cs b/gclo/EditableComboBox.cs new file mode 100644 index 0000000..0e77827 --- /dev/null +++ b/gclo/EditableComboBox.cs @@ -0,0 +1,60 @@ +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Media; + +namespace gclo +{ + /// + /// Workaround for a WinUI platform quirk: an editable ComboBox never renders + /// Text that was bound before its template loaded — the value sits in the Text + /// property but the template's inner TextBox stays empty, so a seeded value + /// (an account workspace's organization) looks blank. Re-assigning Text is an + /// identical-value no-op, so the inner TextBox must be written directly. + /// + internal static class EditableComboBox + { + /// + /// Pushes into the combo's template TextBox (and the + /// Text property, for good measure). Call from the control's Loaded handler; + /// enqueued once when the template has not been applied yet at that point. + /// + public static void ReapplyText(ComboBox combo, string text) + { + if (FindInnerTextBox(combo) is TextBox inner) + { + inner.Text = text; + inner.SelectionStart = text.Length; + } + else + { + // Template not applied yet: try once more after this layout pass. + combo.DispatcherQueue.TryEnqueue(() => + { + if (FindInnerTextBox(combo) is TextBox late) + { + late.Text = text; + late.SelectionStart = text.Length; + } + }); + } + } + + private static TextBox? FindInnerTextBox(DependencyObject root) + { + int count = VisualTreeHelper.GetChildrenCount(root); + for (int i = 0; i < count; i++) + { + DependencyObject child = VisualTreeHelper.GetChild(root, i); + if (child is TextBox box) + { + return box; + } + if (FindInnerTextBox(child) is TextBox nested) + { + return nested; + } + } + return null; + } + } +} diff --git a/gclo/WorkspacePage.xaml b/gclo/WorkspacePage.xaml index 18fe1bc..dc4f0dd 100644 --- a/gclo/WorkspacePage.xaml +++ b/gclo/WorkspacePage.xaml @@ -104,6 +104,7 @@ IsEditable="True" ItemsSource="{x:Bind ViewModel.Organizations, Mode=OneTime}" PlaceholderText="Paste a token first, or type an org" + Loaded="OrgBox_Loaded" Text="{x:Bind ViewModel.Organization, Mode=TwoWay}" /> "Filter by archived", }; + // An editable ComboBox does not render Text that was set before its template + // loaded, so an account workspace's seeded org shows as blank even though + // the view model holds it — see EditableComboBox. Runs on every Loaded: + // the connect card's box on page creation, the chip flyout's on each open. + private void OrgBox_Loaded(object sender, RoutedEventArgs e) + => EditableComboBox.ReapplyText((ComboBox)sender, ViewModel.Organization); + private void Toolbar_SizeChanged(object sender, SizeChangedEventArgs e) => UpdateToolbarLayout(); -- 2.54.0 From 94819f7773d5928f47cab6254560ce54380124ef Mon Sep 17 00:00:00 2001 From: James Maes Date: Sun, 5 Jul 2026 07:57:36 -0500 Subject: [PATCH 2/3] Settings: default PAT (vault), folder picker, splash controls; branded About MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Default GitHub token: stored only in Windows Credential Manager under a well-known vault id; the Settings box stays empty ("unchanged") rather than re-materializing the secret, with an explicit remove link. Quick Sync pre-fills its token from it, which also starts the org lookup. - Default target folder now has a Browse... folder picker like the workspace and wizard. - Advanced section: splash on/off and duration (500-30000 ms, default 5000, clamped on save — tests added). MainWindow honors both. - About dialog opens with the splash's brand header (gradient, glyph, wordmark, version) instead of a plain text block. Co-Authored-By: Claude Fable 5 --- gclo.Engine.Tests/AppSettingsTests.cs | 49 +++++++++++++++ gclo.ViewModels/AppSettings.cs | 18 ++++++ gclo/AboutDialog.xaml | 54 ++++++++++++---- gclo/MainWindow.xaml.cs | 38 +++++++---- gclo/SettingsDialog.xaml | 69 +++++++++++++++++--- gclo/SettingsDialog.xaml.cs | 91 +++++++++++++++++++++++---- 6 files changed, 277 insertions(+), 42 deletions(-) create mode 100644 gclo.Engine.Tests/AppSettingsTests.cs diff --git a/gclo.Engine.Tests/AppSettingsTests.cs b/gclo.Engine.Tests/AppSettingsTests.cs new file mode 100644 index 0000000..b755d1c --- /dev/null +++ b/gclo.Engine.Tests/AppSettingsTests.cs @@ -0,0 +1,49 @@ +using gclo.ViewModels; + +namespace gclo.Engine.Tests; + +/// Pins defaults and load-time sanitizing of . +public sealed class AppSettingsTests +{ + [Fact] + public void Defaults_SplashOnAtFiveSeconds() + { + var settings = new AppSettings(); + + Assert.True(settings.ShowSplashScreen); + Assert.Equal(5_000, settings.SplashMilliseconds); + } + + [Fact] + public void Save_ClampsSplashDuration() + { + string dir = Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + Environment.SetEnvironmentVariable("GCLO_DATA_DIR", dir); + try + { + var settings = new AppSettings { SplashMilliseconds = 999_999 }; + settings.Save(); + Assert.Equal(AppSettings.MaxSplashMilliseconds, settings.SplashMilliseconds); + + settings.SplashMilliseconds = 1; + settings.Save(); + Assert.Equal(AppSettings.MinSplashMilliseconds, settings.SplashMilliseconds); + + // Round trip: the persisted values load back as saved. + var loaded = AppSettings.Load(); + Assert.Equal(AppSettings.MinSplashMilliseconds, loaded.SplashMilliseconds); + Assert.True(loaded.ShowSplashScreen); + } + finally + { + Environment.SetEnvironmentVariable("GCLO_DATA_DIR", null); + try + { + Directory.Delete(dir, recursive: true); + } + catch (IOException) + { + } + } + } +} diff --git a/gclo.ViewModels/AppSettings.cs b/gclo.ViewModels/AppSettings.cs index 3692af1..413c70e 100644 --- a/gclo.ViewModels/AppSettings.cs +++ b/gclo.ViewModels/AppSettings.cs @@ -15,6 +15,17 @@ public sealed class AppSettings public const int MaxConcurrency = 64; public const int DefaultConcurrency = 8; + public const int MinSplashMilliseconds = 500; + public const int MaxSplashMilliseconds = 30_000; + public const int DefaultSplashMilliseconds = 5_000; + + /// + /// Vault id under which the optional default GitHub token is stored. The token + /// itself lives only in the (Credential Manager) — + /// never in this settings file. + /// + public static readonly Guid DefaultTokenVaultId = new("d0f0e0c0-9c40-4a5e-8f3a-5b1e6f7a2c11"); + /// Folder pre-filled as the sync target. Empty means "no default". public string DefaultTargetFolder { get; set; } = ""; @@ -24,6 +35,12 @@ public sealed class AppSettings /// App theme: "System", "Light", or "Dark". Unknown values fall back to "System". public string Theme { get; set; } = "System"; + /// Whether the branded splash overlay shows at startup. + public bool ShowSplashScreen { get; set; } = true; + + /// How long the splash stays up before fading, clamped to 500..30000 ms. + public int SplashMilliseconds { get; set; } = DefaultSplashMilliseconds; + private static string SettingsPath => Path.Combine(GcloPaths.DataRoot, "settings.json"); /// @@ -84,6 +101,7 @@ public sealed class AppSettings // The JSON deserializer can assign null despite the non-nullable annotation. DefaultTargetFolder ??= ""; DefaultMaxConcurrency = Math.Clamp(DefaultMaxConcurrency, MinConcurrency, MaxConcurrency); + SplashMilliseconds = Math.Clamp(SplashMilliseconds, MinSplashMilliseconds, MaxSplashMilliseconds); if (Theme is not ("System" or "Light" or "Dark")) { Theme = "System"; diff --git a/gclo/AboutDialog.xaml b/gclo/AboutDialog.xaml index 869b78c..4765f19 100644 --- a/gclo/AboutDialog.xaml +++ b/gclo/AboutDialog.xaml @@ -10,20 +10,50 @@ CloseButtonText="Close" DefaultButton="Close"> - + - - - - - - + + + + + + + + + + + + + + + + + + + DisposeWorkspaces(); - // Dismiss the startup splash overlay once it has been visible long enough - // to read as an intentional brand moment rather than a flicker. - DispatcherQueue.TryEnqueue(async () => + // The splash overlay honors Settings → Advanced: skipped entirely when + // disabled, otherwise dismissed after the configured display time. + if (_settings.ShowSplashScreen) { - await Task.Delay(MinimumSplashMilliseconds); - await StartupSplash.DismissAsync(); - }); + DispatcherQueue.TryEnqueue(async () => + { + await Task.Delay(_settings.SplashMilliseconds); + await StartupSplash.DismissAsync(); + }); + } + else + { + StartupSplash.Visibility = Visibility.Collapsed; + } } - /// Minimum splash display, so a fast startup never shows a flicker. - private const int MinimumSplashMilliseconds = 800; - private void ApplySettings() { if (Content is FrameworkElement root) @@ -253,8 +257,14 @@ namespace gclo { if (id == Guid.Empty) { - // Quick Sync: no account behind it — exactly the pre-accounts behavior. - return new WorkspaceViewModel(log: _log); + // Quick Sync: no account behind it. A saved default token (Settings) + // pre-fills the connect card; setting Token also starts the org lookup. + var quickSync = new WorkspaceViewModel(log: _log); + if (_tokenVault.TryRetrieve(AppSettings.DefaultTokenVaultId) is { Length: > 0 } defaultToken) + { + quickSync.Token = defaultToken; + } + return quickSync; } Account? account = _accountsStore.GetAll().FirstOrDefault(a => a.Id == id); @@ -662,7 +672,11 @@ namespace gclo private async void SettingsMenuItem_Click(object sender, RoutedEventArgs e) { - var dialog = new SettingsDialog(_settings) { XamlRoot = Content.XamlRoot }; + var dialog = new SettingsDialog( + _settings, _tokenVault, () => WinRT.Interop.WindowNative.GetWindowHandle(this)) + { + XamlRoot = Content.XamlRoot, + }; if (await DialogGuard.ShowAsync(dialog) == ContentDialogResult.Primary) { dialog.ApplyAndSave(); diff --git a/gclo/SettingsDialog.xaml b/gclo/SettingsDialog.xaml index 46e943e..1f1bdba 100644 --- a/gclo/SettingsDialog.xaml +++ b/gclo/SettingsDialog.xaml @@ -11,25 +11,80 @@ CloseButtonText="Cancel" DefaultButton="Primary"> - - + + + + + + + +