From 6ce1cf13fe534e03fd89369163fefc9dbde15e5c Mon Sep 17 00:00:00 2001 From: RghvGrv Date: Fri, 14 Aug 2026 03:02:27 +0530 Subject: [PATCH] fix(mobile): load tab screens once, not on every tab switch Shell raises OnAppearing on every tab selection, and all five tab pages called LoadCommand there unconditionally - so each tab tap cost a full API round trip, cleared the collections and repainted them. Against a slow or unreachable API that is a spinner every time the user comes back to a screen, which reads as an app permanently refreshing itself. Each page's OnAppearing now calls EnsureLoadedCommand: fetch on the first visit, then leave the screen alone. Freshness comes from invalidation rather than repetition: - SubscriptionsChangedMessage marks the list stale, so the reload happens when the user is looking at that screen instead of behind their back. The dashboard, a singleton whose totals move with any write, refetches straight away as before. - Category and payment-source renames and deletes now publish that message too. The dashboard names both in its breakdowns and no longer refetches on tab switch, so the change has to be announced. - SessionEndedMessage clears each screen and re-arms its fetch. Shell keeps the page it built for each ShellContent, so without this the next user to sign in would be handed the previous one's rows and no request would follow. - Pull-to-refresh is unchanged: RefreshView still binds LoadCommand, which always fetches. - A load that ended with nothing on screen is not "loaded", so an error screen still retries on the next visit. Cached rows do count, which is the case the refetch made painful. Also brings the Payment rows in line with Categories after #179: the "..." button is the same vertical-dots tonal icon button, and the SwipeView is gone. Two ways into one menu is one more than the screen needs, and only one of them was ever discoverable. --- docs/Design.md | 1 + .../ViewModels/CategoriesViewModel.cs | 49 ++- .../ViewModels/DashboardViewModel.cs | 23 ++ .../ViewModels/PaymentSourcesViewModel.cs | 49 ++- .../ViewModels/SettingsViewModel.cs | 30 +- .../ViewModels/SubscriptionListViewModel.cs | 49 +++ .../Views/CategoriesPage.xaml.cs | 4 +- .../Views/DashboardPage.xaml.cs | 4 +- .../Views/PaymentSourcesPage.xaml | 85 ++--- .../Views/PaymentSourcesPage.xaml.cs | 8 +- src/SubVora.Mobile/Views/SettingsPage.xaml.cs | 4 +- .../Views/SubscriptionListPage.xaml.cs | 4 +- .../CategoriesViewModelTests.cs | 9 +- .../CategoryEditingViewModelTests.cs | 5 +- .../CategoryGroupingTests.cs | 5 +- .../Fakes/FakeSubscriptionsApi.cs | 9 +- .../SubVora.Mobile.Tests/ManageActionTests.cs | 7 +- .../OfflineFailureTests.cs | 6 +- .../OfflineWriteGuardTests.cs | 8 +- .../PaymentSourcesViewModelTests.cs | 3 +- .../TabSwitchReloadTests.cs | 346 ++++++++++++++++++ 21 files changed, 630 insertions(+), 78 deletions(-) create mode 100644 tests/SubVora.Mobile.Tests/TabSwitchReloadTests.cs diff --git a/docs/Design.md b/docs/Design.md index b2dec95..99fb861 100644 --- a/docs/Design.md +++ b/docs/Design.md @@ -57,6 +57,7 @@ The application automates mundane workflows like logo provisioning, smart catego 1. **Cross-Platform Mobile Component (.NET MAUI):** * **Local State Caching:** Embedded `SQLite` context database providing sub-second runtime latency and offline access capabilities. * **Renewal Reminders:** Local notifications scheduled on-device from the local mirror. The OS delivers them with the app closed, so no push service, vendor project or API key is involved. + * **Screens Load Once, Not Per Tab Switch:** Shell raises `OnAppearing` on every tab selection, so each page's `OnAppearing` calls `EnsureLoadedCommand` — a first-visit fetch, then nothing. Freshness comes from invalidation, not repetition: a write publishes `SubscriptionsChangedMessage`, signing out publishes `SessionEndedMessage`, and pull-to-refresh (`LoadCommand`) always fetches. A load that ended with nothing on screen is not "loaded", so an error screen still retries on the next visit. 2. **Microservice Backend API (ASP.NET Core):** * **Authentication Matrix:** Secure stateless JWT (JSON Web Tokens) handling verification flows via industry-grade encryption frameworks. * **Background Work:** Only what a client cannot do for itself — refreshing cached FX rates, syncing the provider catalog, and dispatching queued email. Nothing advances a billing date on a timer: that moves when the user marks a charge paid, so a date left in the past means the charge is genuinely outstanding. diff --git a/src/SubVora.Mobile/ViewModels/CategoriesViewModel.cs b/src/SubVora.Mobile/ViewModels/CategoriesViewModel.cs index 7dd7ee6..01b335a 100644 --- a/src/SubVora.Mobile/ViewModels/CategoriesViewModel.cs +++ b/src/SubVora.Mobile/ViewModels/CategoriesViewModel.cs @@ -1,9 +1,11 @@ using System.Collections.ObjectModel; using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; +using CommunityToolkit.Mvvm.Messaging; using Refit; using SubVora.Mobile.Api; using SubVora.Mobile.Api.Dtos; +using SubVora.Mobile.Messages; using SubVora.Mobile.Services; namespace SubVora.Mobile.ViewModels; @@ -13,6 +15,15 @@ public partial class CategoriesViewModel : ObservableObject private readonly ICategoriesApi _categoriesApi; private readonly IConnectivityService _connectivity; private readonly IUserPrompt _userPrompt; + private readonly IMessenger _messenger; + + /// + /// Whether this screen already holds the category list. Shell raises OnAppearing on every tab + /// selection, so loading unconditionally there refetched on each tab tap - see + /// SubscriptionListViewModel._isLoaded for the full reasoning. Every mutation below + /// applies its own result locally, so nothing on this screen goes stale between visits. + /// + private bool _isLoaded; [ObservableProperty] public partial bool IsLoading { get; set; } @@ -37,12 +48,38 @@ public partial class CategoriesViewModel : ObservableObject /// public ObservableCollection Groups { get; } = []; - public CategoriesViewModel(ICategoriesApi categoriesApi, IConnectivityService connectivity, IUserPrompt userPrompt) + public CategoriesViewModel( + ICategoriesApi categoriesApi, + IConnectivityService connectivity, + IUserPrompt userPrompt, + IMessenger messenger) { _categoriesApi = categoriesApi; _connectivity = connectivity; _userPrompt = userPrompt; + _messenger = messenger; IsOffline = !connectivity.IsConnected; + + // Weak registration: the messenger is a singleton and this view model is not. + messenger.Register(this, (_, _) => Reset()); + } + + /// + /// What OnAppearing calls: load on the first visit only. See . + /// + [RelayCommand] + private Task EnsureLoadedAsync() => _isLoaded ? Task.CompletedTask : LoadAsync(); + + /// + /// Drops the signed-out session's categories. Shell keeps the page it built for each tab, so + /// without this the next user would see the previous one's list and no fetch would follow. + /// + private void Reset() + { + _isLoaded = false; + Categories.Clear(); + Groups.Clear(); + ErrorMessage = null; } /// @@ -78,9 +115,11 @@ private async Task LoadAsync() } RebuildGroups(); + _isLoaded = true; } catch (Exception ex) when (ApiErrorMapper.IsApiFailure(ex)) { + // _isLoaded stays false: there is nothing on screen, so the next visit should retry. ErrorMessage = ApiErrorMapper.ToDisplayMessage(ex); } finally @@ -208,6 +247,10 @@ private async Task RenameAsync(CategoryDto category) Categories[index] = renamed; RebuildGroups(); } + + // The dashboard names categories in its breakdown and the subscription list groups by + // them, and neither refetches on tab switch any more - so the rename has to say so. + _messenger.Send(new SubscriptionsChangedMessage()); } catch (Exception ex) when (ApiErrorMapper.IsApiFailure(ex)) { @@ -243,6 +286,10 @@ private async Task DeleteAsync(CategoryDto category) Categories.Remove(category); RebuildGroups(); + // Subscriptions that used it are now uncategorised, which moves both the dashboard + // breakdown and the grouping on the list. + _messenger.Send(new SubscriptionsChangedMessage()); + if (result.SubscriptionsUncategorized > 0) { var plural = result.SubscriptionsUncategorized == 1 ? "subscription is" : "subscriptions are"; diff --git a/src/SubVora.Mobile/ViewModels/DashboardViewModel.cs b/src/SubVora.Mobile/ViewModels/DashboardViewModel.cs index 5222c06..cf24742 100644 --- a/src/SubVora.Mobile/ViewModels/DashboardViewModel.cs +++ b/src/SubVora.Mobile/ViewModels/DashboardViewModel.cs @@ -74,6 +74,15 @@ public partial class DashboardViewModel : ObservableObject public ObservableCollection ByPaymentSource { get; } = []; + /// + /// Whether the figures on screen came from a load. Shell raises OnAppearing on every tab + /// selection, so loading unconditionally there meant a fetch per tab tap - see + /// SubscriptionListViewModel._isLoaded for the full reasoning. Here the message handler + /// below refetches straight away rather than only marking it stale, because this view model is + /// a singleton whose totals are what a change to a subscription or the home currency moves. + /// + private bool _isLoaded; + public DashboardViewModel(IDashboardApi dashboardApi, ILocalCacheService localCacheService, IMessenger messenger) { _dashboardApi = dashboardApi; @@ -85,12 +94,22 @@ public DashboardViewModel(IDashboardApi dashboardApi, ILocalCacheService localCa messenger.Register(this, (_, _) => Clear()); } + /// + /// What OnAppearing calls: load the first time the tab is opened, then leave the numbers alone + /// until a change message refetches them. See . + /// + [RelayCommand] + private Task EnsureLoadedAsync() => _isLoaded ? Task.CompletedTask : LoadAsync(); + /// /// Drops the figures so the banner cannot outlive the session that produced them - a signed-out /// or expired user must not still see their spend on the login screen. /// public void Clear() { + // Also makes the next appearance fetch again: this view model is a singleton, so without it + // the next user to sign in would open a dashboard that believes it is already loaded. + _isLoaded = false; Weekly = 0; Monthly = 0; Yearly = 0; @@ -130,6 +149,7 @@ private async Task LoadAsync() ApplyBurnRate(snapshot); IsShowingCachedData = false; + _isLoaded = true; await _localCacheService.UpsertAsync(snapshot); } @@ -143,6 +163,9 @@ private async Task LoadAsync() { ApplyBurnRate(cached); IsShowingCachedData = true; + + // Cached totals still count as loaded - see the same call in the list view model. + _isLoaded = true; } else { diff --git a/src/SubVora.Mobile/ViewModels/PaymentSourcesViewModel.cs b/src/SubVora.Mobile/ViewModels/PaymentSourcesViewModel.cs index 133a5fd..e721c19 100644 --- a/src/SubVora.Mobile/ViewModels/PaymentSourcesViewModel.cs +++ b/src/SubVora.Mobile/ViewModels/PaymentSourcesViewModel.cs @@ -1,9 +1,11 @@ using System.Collections.ObjectModel; using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; +using CommunityToolkit.Mvvm.Messaging; using Refit; using SubVora.Mobile.Api; using SubVora.Mobile.Api.Dtos; +using SubVora.Mobile.Messages; using SubVora.Mobile.Services; namespace SubVora.Mobile.ViewModels; @@ -13,6 +15,15 @@ public partial class PaymentSourcesViewModel : ObservableObject private readonly IPaymentSourcesApi _paymentSourcesApi; private readonly IUserPrompt _userPrompt; private readonly IConnectivityService _connectivity; + private readonly IMessenger _messenger; + + /// + /// Whether this screen already holds the payment sources. Shell raises OnAppearing on every tab + /// selection, so loading unconditionally there refetched on each tab tap - see + /// SubscriptionListViewModel._isLoaded for the full reasoning. Every mutation below + /// applies its own result locally, so the list cannot go stale between visits. + /// + private bool _isLoaded; [ObservableProperty] public partial bool IsLoading { get; set; } @@ -30,12 +41,37 @@ public partial class PaymentSourcesViewModel : ObservableObject public ObservableCollection PaymentSources { get; } = []; - public PaymentSourcesViewModel(IPaymentSourcesApi paymentSourcesApi, IUserPrompt userPrompt, IConnectivityService connectivity) + public PaymentSourcesViewModel( + IPaymentSourcesApi paymentSourcesApi, + IUserPrompt userPrompt, + IConnectivityService connectivity, + IMessenger messenger) { _connectivity = connectivity; IsOffline = !connectivity.IsConnected; _paymentSourcesApi = paymentSourcesApi; _userPrompt = userPrompt; + _messenger = messenger; + + // Weak registration: the messenger is a singleton and this view model is not. + messenger.Register(this, (_, _) => Reset()); + } + + /// + /// What OnAppearing calls: load on the first visit only. See . + /// + [RelayCommand] + private Task EnsureLoadedAsync() => _isLoaded ? Task.CompletedTask : LoadAsync(); + + /// + /// Drops the signed-out session's payment sources. Shell keeps the page it built for each tab, + /// so without this the next user would see the previous one's accounts and no fetch would run. + /// + private void Reset() + { + _isLoaded = false; + PaymentSources.Clear(); + ErrorMessage = null; } /// /// Whether the device has no network. Refreshed when the screen loads and after a failed write @@ -69,9 +105,12 @@ private async Task LoadAsync() { PaymentSources.Add(paymentSource); } + + _isLoaded = true; } catch (Exception ex) when (ApiErrorMapper.IsApiFailure(ex)) { + // _isLoaded stays false: nothing is on screen, so the next visit should retry. // A read, so the plain wording: nothing was lost, there is just nothing to show. IsOffline = !_connectivity.IsConnected; ErrorMessage = ApiErrorMapper.ToDisplayMessage(ex); @@ -125,6 +164,10 @@ private async Task DeleteAsync(Guid id) { PaymentSources.Remove(toRemove); } + + // Subscriptions that billed to it are detached, which changes the dashboard's spend-by- + // account breakdown. Neither that screen nor the list refetches on tab switch now. + _messenger.Send(new SubscriptionsChangedMessage()); } catch (Exception ex) when (ApiErrorMapper.IsApiFailure(ex)) { @@ -167,6 +210,10 @@ private async Task RenameAsync(PaymentSourceDto paymentSource) PaymentSources.RemoveAt(index); PaymentSources.Insert(SortedIndexFor(updated.Label), updated); } + + // The dashboard names accounts in its spend-by-account summary, and it no longer + // refetches on tab switch - so the new label has to be announced. + _messenger.Send(new SubscriptionsChangedMessage()); } catch (Exception ex) when (ApiErrorMapper.IsApiFailure(ex)) { diff --git a/src/SubVora.Mobile/ViewModels/SettingsViewModel.cs b/src/SubVora.Mobile/ViewModels/SettingsViewModel.cs index 740ca8c..0bfe3a4 100644 --- a/src/SubVora.Mobile/ViewModels/SettingsViewModel.cs +++ b/src/SubVora.Mobile/ViewModels/SettingsViewModel.cs @@ -1,7 +1,7 @@ using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; -using Refit; using CommunityToolkit.Mvvm.Messaging; +using Refit; using SubVora.Mobile.Api; using SubVora.Mobile.Api.Dtos; using SubVora.Mobile.Formatting; @@ -128,6 +128,32 @@ public SettingsViewModel(IUsersApi usersApi, IAuthApi authApi, ITokenStore token // OnThemeChanged and re-applies the value it already has, which is a no-op - the theme was // applied at startup, long before this page is built. Theme = _themeService.Current; + + // Weak registration: the messenger is a singleton and this view model is not. Signing out + // is published from this same view model, and the handler is what makes the next user's + // visit fetch their profile instead of showing the previous one's. + messenger.Register(this, (_, _) => Reset()); + } + + /// + /// Whether the profile has already been fetched. Shell raises OnAppearing on every tab + /// selection, so loading unconditionally there meant a request per tab tap - see + /// SubscriptionListViewModel._isLoaded for the full reasoning. Save applies what the + /// server returns, so the fields cannot drift from it between visits. + /// + private bool _isLoaded; + + /// What OnAppearing calls: fetch the profile on the first visit only. + [RelayCommand] + private Task EnsureLoadedAsync() => _isLoaded ? Task.CompletedTask : LoadAsync(); + + /// Drops the signed-out session's profile so the next one is fetched, not inherited. + private void Reset() + { + _isLoaded = false; + PreferredCurrency = string.Empty; + SelectedCurrency = null; + ErrorMessage = null; } /// @@ -162,9 +188,11 @@ private async Task LoadAsync() Currencies = SupportedCurrencies.Including(profile.PreferredCurrency); PreferredCurrency = profile.PreferredCurrency; DefaultAlertDaysAdvance = profile.DefaultAlertDaysAdvance; + _isLoaded = true; } catch (Exception ex) when (ApiErrorMapper.IsApiFailure(ex)) { + // _isLoaded stays false: the fields hold nothing, so the next visit should retry. ErrorMessage = ApiErrorMapper.ToDisplayMessage(ex); } finally diff --git a/src/SubVora.Mobile/ViewModels/SubscriptionListViewModel.cs b/src/SubVora.Mobile/ViewModels/SubscriptionListViewModel.cs index ee0a2c9..b74928c 100644 --- a/src/SubVora.Mobile/ViewModels/SubscriptionListViewModel.cs +++ b/src/SubVora.Mobile/ViewModels/SubscriptionListViewModel.cs @@ -79,6 +79,23 @@ public partial class SubscriptionListViewModel : ObservableObject /// Raised by the Add toolbar button, to navigate to the detail screen in add mode. public event EventHandler? AddRequested; + /// + /// Whether this screen already holds rows worth showing. + /// + /// Shell raises OnAppearing on every tab selection, so a page that loads unconditionally there + /// refetches - clearing and repainting itself - on each tab tap. Against a slow or unreachable + /// API that is a spinner every time the user comes back, which is what it looked like the app + /// was stuck refreshing. + /// + /// + /// Invalidated by rather than by a clock: this data + /// only moves when this app moves it, and every writer already publishes that message. Failing + /// with nothing to show leaves the flag false, so a retry still happens on the next visit, and + /// pull-to-refresh forces one through at any time. + /// + /// + private bool _isLoaded; + public SubscriptionListViewModel( ISubscriptionsApi subscriptionsApi, ILocalCacheService localCacheService, @@ -95,6 +112,33 @@ public SubscriptionListViewModel( _connectivity = connectivity; IsOffline = !connectivity.IsConnected; + + // Weak registrations (WeakReferenceMessenger), so the singleton messenger does not keep a + // transient view model alive. A write made from the detail screen marks the list stale; the + // reload happens when the user is actually looking at it rather than behind their back. + messenger.Register(this, (_, _) => _isLoaded = false); + messenger.Register(this, (_, _) => Reset()); + } + + /// + /// What OnAppearing calls: load on the first visit, then leave the screen alone until something + /// says the data moved. See . + /// + [RelayCommand] + private Task EnsureLoadedAsync() => _isLoaded ? Task.CompletedTask : LoadAsync(); + + /// + /// Drops the signed-out session's rows. The tab pages outlive a sign-out - Shell keeps the page + /// it built for each ShellContent - so without this the next user would be handed the previous + /// one's list on the first appearance and no fetch, because the screen thinks it is loaded. + /// + private void Reset() + { + _isLoaded = false; + Subscriptions.Clear(); + Groups.Clear(); + ErrorMessage = null; + IsShowingCachedData = false; } [RelayCommand] @@ -114,6 +158,7 @@ private async Task LoadAsync() } IsShowingCachedData = false; + _isLoaded = true; await _localCacheService.ClearAsync(); foreach (var subscription in result) @@ -136,6 +181,10 @@ private async Task LoadAsync() } IsShowingCachedData = true; + + // Cached rows count as loaded: showing the mirror and refetching on every tab tap + // is the same 30-second spinner, for a screen that already has something on it. + _isLoaded = true; } else { diff --git a/src/SubVora.Mobile/Views/CategoriesPage.xaml.cs b/src/SubVora.Mobile/Views/CategoriesPage.xaml.cs index d6195f3..087bf6d 100644 --- a/src/SubVora.Mobile/Views/CategoriesPage.xaml.cs +++ b/src/SubVora.Mobile/Views/CategoriesPage.xaml.cs @@ -20,7 +20,9 @@ public CategoriesPage(CategoriesViewModel viewModel, IUserPrompt userPrompt) protected override void OnAppearing() { base.OnAppearing(); - _viewModel.LoadCommand.Execute(null); + // EnsureLoaded, not Load: Shell raises OnAppearing on every tab selection, and a + // refetch per tab tap is what made the app look like it was permanently refreshing. + _viewModel.EnsureLoadedCommand.Execute(null); } /// diff --git a/src/SubVora.Mobile/Views/DashboardPage.xaml.cs b/src/SubVora.Mobile/Views/DashboardPage.xaml.cs index 1dd9c1a..3c94ccc 100644 --- a/src/SubVora.Mobile/Views/DashboardPage.xaml.cs +++ b/src/SubVora.Mobile/Views/DashboardPage.xaml.cs @@ -16,6 +16,8 @@ public DashboardPage(DashboardViewModel viewModel) protected override void OnAppearing() { base.OnAppearing(); - _viewModel.LoadCommand.Execute(null); + // EnsureLoaded, not Load: Shell raises OnAppearing on every tab selection, and a + // refetch per tab tap is what made the app look like it was permanently refreshing. + _viewModel.EnsureLoadedCommand.Execute(null); } } diff --git a/src/SubVora.Mobile/Views/PaymentSourcesPage.xaml b/src/SubVora.Mobile/Views/PaymentSourcesPage.xaml index 922c73c..794d589 100644 --- a/src/SubVora.Mobile/Views/PaymentSourcesPage.xaml +++ b/src/SubVora.Mobile/Views/PaymentSourcesPage.xaml @@ -52,55 +52,44 @@ - - - - - - - + + + + + +