fix: Apps settings connection badge never updates after connect/disconnect - #404
Merged
Conversation
NavigationBar fetched the apps list once on mount and never again, so AppsDialog's connection badge stayed stale for the lifetime of the mount even though OAuthAppSettings connected/disconnected correctly underneath. Subscribe to AppsService's 'connected'/'disconnected' events and refetch on each, so the badge updates live without a remount. Also removed the dead-code AppsSettings/AppsSettingsView components (unreferenced by any live screen, confirmed via grep) which carried the same staleness bug in their own onBack path. Kept AppsSettings/types.ts since AppDisplayProps is still consumed by AppsDialog and NavigationBar. Fixes FIXES_BACKLOG item #60.
Repo-owner review on PR #404: having NavigationBar subscribe to AppsService and pass a refreshed `apps` array down to AppsDialog created an unwanted dependency between NavigationBar and the Apps-settings feature. AppsDialog should own its own service subscription instead. Splits AppsDialog into a thin smart AppsDialog.tsx (fetches apps via appsService.openSettings(), subscribes to 'connected'/'disconnected', no incyclist-services import in the view) and a pure AppsDialogView.tsx carrying the original presentational logic, mirroring the live WorkoutDetailsDialog/WorkoutDetailsView and NavigationBar/NavigationBarView split already used in this codebase. Storybook (AppsDialog.stories.tsx) now targets AppsDialogView directly with plain mock apps, same pattern as NavigationBar.stories.tsx targeting NavigationBarView - no service mocking needed. NavigationBar.tsx reverts to rendering <AppsDialog /> without owning any apps state or AppsService subscription itself. The subscription-behavior test moves from NavigationBar.test.tsx to AppsDialog.test.tsx; the former now only asserts NavigationBar renders AppsDialog without an `apps` prop.
…ipt:S6481)
AppsDialogView.tsx:81 created a fresh { standalone: false } object literal
on every render inside a .map() loop, where useMemo can't be used without
violating Rules of Hooks. Hoisted to a module-level constant instead.
|
gdoumen
marked this pull request as ready for review
August 19, 2026 11:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NavigationBarfetched theappsarray once on mount (refInitializedguard) and passed it toAppsDialog, which derives its connection badge purely from that array. Nothing propagated connect/disconnect changes back up toNavigationBar, so the badge stayed stale until the screen remounted (navigation or restart).NavigationBarnow subscribes toAppsService's existing'connected'/'disconnected'events (services/src/apps/service.ts:59,76) and refetchesappson each, so the badge updates live within the same dialog session — no remount required.AppsSettings/AppsSettingsViewcomponents (confirmed unreferenced by any live navigation screen via grep — only theirAppDisplayProps/AppsSettingsViewProps/AppsSettingsPropstypes were referenced elsewhere). They carried the identical staleness bug in their ownonBackpath. KeptAppsSettings/types.ts, trimmed to justAppDisplayProps, sinceAppsDialogandNavigationBarstill depend on that type.NavigationBar.test.tsx(didn't exist before) covering: fetch + subscribe on mount,appsstate updates on'connected',appsstate updates on'disconnected', and unsubscribe on unmount.Refs
FIXES_BACKLOG.mditem #60.Test plan
npx jest— full suite: 110 suites / 791 tests passingnpx eslint src/— 0 errors (pre-existing warnings only, none in touched files)npx tsc --noEmit— clean