Unify mobile loading spinners

[klopez4212]

What

• add the shared desktop-style arc spinner for mobile
• replace app loading indicators with the shared component
• preserve a static pose when reduced motion is enabled

Stack

• follows Refine mobile attachment picking #3313

Validation

• just mobile-check
• focused spinner and pairing widget tests

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c60393eb6d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[klopez4212]

🤖 ### Shared mobile loading indicator
The same compact ring loader now replaces the mixed loading treatments throughout the mobile app.

[klopez4212]

🤖 @codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38be70c193

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41621f55c2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Wrap the spinner test in a ProviderScope

When either test pumps _testable, the tree starts at MaterialApp, but BuzzLoadingIndicator is now a HookConsumerWidget. Riverpod consumer elements require a ProviderScope ancestor, so this focused test throws “No ProviderScope found” before reaching its animation or semantics assertions and fails the mobile test suite. Wrap the tree with WidgetHelpers.testable() or add a custom ProviderScope around the MaterialApp.

AGENTS.md reference: AGENTS.md:L583-L584

Useful? React with 👍 / 👎.

[wesbillman]

Correction after exact-head verification: this is not a failure under the pinned Riverpod 3.1.0 behavior. BuzzLoadingIndicator never reads ref, so no container lookup occurs; the focused file runs without ProviderScope and both tests pass (+2: All tests passed). Retracting this finding.

[brow]

🤖 Reviewed at 41621f55c2ef3a123c44baf0910fa12397f8f968. One real finding, two lines. Everything else in the refactor checks out: no determinate spinner was replaced with an indeterminate one, no condition or centering changed, pairing_page_test.dart adapted the assertion rather than weakening it, the animation controller is disposed correctly, and the 18 sites where the rendered diameter grows were probed at production geometry with no overflow or clipping anywhere. flutter test is 832 passed / 1 skipped / 0 failed, flutter analyze clean, and the new buzz_loading_indicator_test.dart was mutation-checked (three mutants, all caught).

Spinner is nearly invisible on the two disabled-button call sites.

compose_note_page.dart:85 and invite_join_sheet.dart:122 pass color: context.colors.onPrimary, but both spinners render inside a FilledButton that is disabled at exactly the moment the spinner is visible (onPressed: hasText && !isSending.value ? submit : null at compose_note_page.dart:73; onPressed: isClaiming || state.requiresFreshInvite ? null : ... at invite_join_sheet.dart:107-110). A disabled M3 FilledButton does not paint primary, it paints onSurface at 12% alpha, so onPrimary lands almost exactly on the button background.

Measured by resolving the actual painted Material.color per accent per mode, 9 accents plus the raw scheme, both modes:

config	after	before
light, default Black accent	1.21:1	17.4:1
dark, default Black accent	1.53:1	9.3:1

12 of the 20 configurations land below 3:1. Every accent fails in dark mode, because a dark scheme's onPrimary is always the dark contrast pick while the disabled background is onSurface at 12% over a dark surface. In light mode 8 of 9 accents are fine. The failing pair above is the default configuration, not a corner case: defaultAccentIndex = 8 is Black (accent_colors.dart), and AccentNotifier.build returns that default for a null, out-of-range, or legacy stored value (theme_provider.dart:50-63), so a fresh install ships it.

Suggested fix: drop the color: argument at both sites and let BuzzLoadingIndicator default to context.colors.primary. That restores exactly what the old code rendered, since app_theme.dart:240-244 already set progressIndicatorTheme.color to scheme.primary. So this is a regression rather than a judgment call.

Worth noting this is not the same situation as send_button.dart:32, where onPrimary is correct, because that button paints context.colors.primary as its own background including the disabled 50%-alpha variant.

One thing we deliberately did not test, flagged so it does not read as cleared: members_sheet.dart:249 grows 10px to 14px inside a ListTile subtitle Row, and it is the one site where the growth consumes horizontal rather than vertical slack next to text. It was probed clean at default text scale only, not at large text scale.

[wesbillman]

Reviewed the spinner-only delta against its stacked base at 41621f55c2ef3a123c44baf0910fa12397f8f968. Requesting changes for one concrete user-facing regression:

P1 — two loading indicators become nearly invisible while their buttons are disabled. mobile/lib/features/pulse/compose_note_page.dart and mobile/lib/features/invites/invite_join_sheet.dart explicitly pass context.colors.onPrimary. In both cases the FilledButton is disabled exactly while its spinner is visible, so Material paints the disabled onSurface-alpha background rather than primary; onPrimary has very poor contrast there, particularly with the default Black accent and in dark mode. The previous CircularProgressIndicator inherited progressIndicatorTheme.color == scheme.primary, which remains visible. Dropping the explicit color at these two sites restores that behavior. (send_button.dart is different because it explicitly retains a primary-colored background.)

Correction: I originally included the unresolved Codex claim that the focused tests require ProviderScope. That claim is false for the pinned Riverpod behavior: because this HookConsumerWidget never reads ref, the consumer element mounts without resolving a container. The exact focused test file was run at this head and passed both tests (+2: All tests passed). I retract that finding.

The shared animation/controller lifecycle, reduced-motion behavior, painter sizing, semantics, focused tests, and the other migrated call sites look sound. The PR is stacked on #3313, whose separate blocker must also be resolved before this stack can merge.

[brow]

🤖 Corrections and additions to my review above, now reviewed by two independent reviewers and re-confirmed at the current head 07c68869f08c3a86aade4088a4a6cc2b3265e2f1. The ask is unchanged: drop the color: argument at compose_note_page.dart:85 and invite_join_sheet.dart:122.

Why this still applies after the force-push. The branch moved 41621f55c → 07c68869f and the base moved to 797014b71, but the change itself is untouched: git diff --shortstat is 28 files changed, 294 insertions(+), 86 deletions(-) for both f1542ee6c..41621f55c and 797014b71..07c68869f, and extracting the added/removed lines from both diffs and sorting them yields identical sets. Every line reference below was re-confirmed at 07c68869f. The measurements were taken at 41621f55c and carried forward on that basis rather than re-run at the new head.

Correction: my earlier dark-mode figure was wrong, and the numbers depend on the accent setting.

I previously published a single pair of ratios. Two reviewers measured this independently and got different numbers, which turned out to be two different theme configurations rather than a disagreement. The cause is applyAccent (mobile/lib/shared/theme/color_scheme.dart:86-94), which returns the base scheme untouched for an out-of-range index and otherwise replaces primary/onPrimary with an accent color and its contrastForeground() pick. Compositing the resolved disabled fill (onSurface at 12.16% over surface) gives:

configuration	disabled fill	new onPrimary	prior primary
default Black accent (index 8)	#E9EAED light	1.20:1	17.46:1
default Black accent (index 8)	#383C51 dark	1.93:1	7.32:1
raw scheme, no accent	#E9EAED light	1.06:1	4.50:1
raw scheme, no accent	#383C51 dark	1.36:1	3.38:1

The first pair is what ships: defaultAccentIndex = 8 is Black, and AccentNotifier.build (theme_provider.dart:52-62) returns it for a null, out-of-range, or legacy stored value, migrating legacyDefaultAccentIndex = -1 to 8 on read. So 1.20:1 light and 1.93:1 dark are the figures for a fresh install, and the dark number supersedes the 1.53:1 I posted earlier (that measurement came from a harness that pumped all configurations in one test body, leaving the resolved button fill stale across theme swaps). Across a 20-configuration sweep of 9 accents plus the raw scheme in both modes, 12 of 20 land below the 3:1 non-text minimum, and every accent fails in dark mode.

Two independent methods agree on the disabled fill: a rendered pixel probe at pixelRatio: 8 over an opaque scheme.surface backing, and a from-source composite of the theme constants.

Why this is a regression rather than a preference. The diff at both sites replaces a bare CircularProgressIndicator(strokeWidth: 2) that passed no color, and app_theme.dart:240-244 sets progressIndicatorTheme.color to scheme.primary. The old code rendered primary; the new code renders onPrimary in the same spot. Deleting the color: argument restores the previous appearance rather than introducing a new choice.

Worth stating the user-visible consequence plainly: in compose, the spinner is the button's only child, so the disabled sending button can appear empty while a post is publishing.

The large-text-scale gap I flagged as untested: it reproduces, and the mechanism is exact.

My earlier comment called out members_sheet.dart:249 as probed at default text scale only. Re-tested against the real MembersSheet (real _MemberTile / _MemberAvatar / ListTile subtree, overriding only channelMembersProvider, workingBotPubkeysProvider, userCacheProvider), A/B by editing line 249 in place and reverting, with identical fixed pumps in both columns:

configuration	old	new
textScale 1.80, width 360	clean	overflowed by 3.6 px
textScale 2.05, width 390	clean	overflowed by 1.6 px

The precise claim: this PR does not create the members-row overflow, it moves the threshold in by 4 px and opens exactly two new windows. Every other configuration that overflows in the new column also overflows in the old one, each time by exactly 4 px more (1.85/360: 5.2 → 9.2; 2.0/375: 7.0 → 11; 1.5/320: 6.0 → 10.0), matching the 10px → 14px diameter delta. Width 390 is clean in both columns at scales 1.0, 1.5, 1.75, 1.80, 1.85 and 2.0.

This is minor and not part of the ask: it needs a text-scale setting above 1.75 on a narrow width, and it clips the Working… label by single-digit pixels. If you want it gone, the row wants Flexible around the text rather than a smaller spinner.

Suggestion, not a request: size and color are currently unasserted.

Two mutations to buzz_loading_indicator.dart leave the full mobile suite at 839 passed / 1 skipped:

• size: Size.square(size) → const Size.square(1) (the widget ignores its size)
• color ?? context.colors.primary → context.colors.primary (the widget ignores its color)

Those are the two parameters carrying all of this PR's per-call-site behavior across 25 sites, so a size/color assertion would be cheap and would have caught the contrast issue. Entirely your call; not part of the requested change.

Everything else checks out. Both reviewers independently confirmed: all 28 replaced indicators were indeterminate, so no determinate progress was lost; loading predicates and centering structure are preserved; every formerly explicit color is retained; app.dart changes only the splash child and leaves auth, routing, providers and deep-link flow untouched; pairing_page_test.dart adapted its assertion rather than weakening it, with the adjacent expect(find.text('Connect'), findsNothing) intact; and the animation controller is disposed correctly by useAnimationController. send_button.dart and the pairing buttons pass onPrimary correctly, because each supplies its own matching background or foreground. flutter test 839 passed / 1 skipped / 0 failed and flutter analyze clean, on both reviewers' independent runs.

[klopez4212]

🤖 Rebased this PR onto current main after #3313 merged and addressed the remaining review feedback on final head bb216fd5b:

• restored readable disabled-button spinner contrast in compose-note and invite-join by using the shared indicator’s theme-aware fallback
• fixed native attachment popover ownership so an in-flight support check no longer swallows another composer’s fallback, and owner disposal always releases ownership even when the native dismissed callback is absent or the bridge is unavailable
• added focused regression coverage for the pending-support, missing-callback, and missing-plugin paths

Validation: Dart format clean, flutter analyze clean, the full compose-bar test file passed, and all 874 mobile tests passed with one expected skip.

@wesbillman @brow please re-review.

[klopez4212]

🤖 @codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb216fd5b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[klopez4212]

🤖 @codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 011f14cde5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[klopez4212]

🤖 @codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ad137ff72

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[klopez4212]

🤖 @codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 02117539f4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Prior requested change is resolved at head 0211753: the two disabled-button spinners now use the theme-aware default color. Re-review found no remaining blocker.

[wesbillman]

Re-reviewed at exact head 02117539f45208c65d2f041bf4f3cc277479f290.

The prior disabled-button contrast blocker is resolved: Compose Note and Invite Join no longer force onPrimary; both use BuzzLoadingIndicator’s theme-aware default primary, restoring the previous visible treatment.

I also reviewed the newly included iOS attachment-popover ownership changes. Pending support checks are cancelled safely across owners, repeated taps by the same owner preserve the in-flight presentation, and disposal clears ownership even if native dismissal fails or never callbacks. Focused regression coverage exercises those paths.

No remaining blocking finding. Mobile CI and all other required checks are green at this exact head. I did not rerun local suites.