Mobile tab bar polish#1368
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9583446a9f
ℹ️ 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".
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Thanks — the tab bar layout math and full mobile suite look good, but I found one accessibility regression in the new moving-pill behavior.
CHANGE: please honor MediaQuery.disableAnimations for the selected-pill slide. _FloatingTabDestination already disables the icon AnimatedSwitcher when reduced motion is requested, but _FloatingTabBar still hard-codes the pill AnimatedAlign to Duration(milliseconds: 180). Since this PR replaces the per-tab active background with a moving selected pill, users with Reduce Motion / disabled animations still get a horizontal slide whenever they switch tabs.
The fix should be small: read final reducedMotion = MediaQuery.of(context).disableAnimations; in _FloatingTabBar too, and use Duration.zero for the AnimatedAlign duration when it is true (matching the icon switcher behavior).
Validation I ran:
PATH="$PWD/bin:$PATH" just mobile-check✅PATH="$PWD/bin:$PATH" just mobile-test✅ (393 passed, 1 skipped)git diff --check pr-1368-base...HEAD✅
|
🤖 Addressed Wes’s requested change in 4efa53e: the floating tab pill now honors |
Resolve the mobile accent color conflict by keeping the PR's Black default accent and current main's dark-mode foreground behavior. Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesb@block.xyz>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Conflict fix looks good, and my prior reduced-motion blocker is addressed.
I verified the final PR diff after merging main is isolated to the mobile tab bar polish, with the accent conflict preserving both Black-as-default and dark-mode foreground contrast behavior.
Validation run locally:
PATH="$PWD/bin:$PATH" dart format --output=none --set-exit-if-changed mobile/lib/shared/theme/accent_colors.dartPATH="$PWD/bin:$PATH" flutter analyze mobile/lib/shared/theme/accent_colors.dart mobile/lib/shared/theme/color_scheme.dart mobile/test/shared/theme/accent_colors_test.dartcd mobile && PATH="$PWD/../bin:$PATH" flutter test test/shared/theme/accent_colors_test.dartPATH="$PWD/bin:$PATH" just mobile-checkPATH="$PWD/bin:$PATH" just mobile-test
GitHub Mobile + DCO are green; one desktop integration shard is still pending at the time of approval.
Resolve home_page.dart conflict: adopt main's context.colors / context.theme / context.textTheme accessors (#1401) while keeping the PR's reduced-motion and selected-index polish. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
2 participants
Summary
Snapshots
Full-width floating tab bar aligned to the app gutters.
Active/inactive icon and label weights.
Checks
cd mobile && ../bin/dart format --output=none --set-exit-if-changed lib/features/home/home_page.dartcd mobile && ../bin/flutter analyze lib/features/home/home_page.dart/private/tmpdisk exhaustion.