Repository navigation
feat(flutter): mobile-first theme and app shell - #111
Conversation
Part A of the #109 split (theme and shell). No screen content changes. - Puls3Breakpoints: one set of named layout breakpoints instead of raw 600/640/700/900/960/1000 numbers across screens and widgets, plus isCompactLayout(). - Phone type scale below 640 px (14 px body, 18-28 px titles), set by the app root; desktop keeps the brand sizes. - Compact visual density and denser inputs on phones; PrimaryButton is 44 px tall there (52 on desktop) inside a 48 px tap target. - Bottom navigation bar on phones (Studio, Marketplace); the top bar keeps the logo and a wallet chip that shrinks instead of overflowing. - ScreenHeader and SkeletonBox building blocks for the following parts. Tests: bottom bar on phones only, compact type and button size, and no overflow on every screen at 320, 360, 390 and 414 px. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TOMOKI977
left a comment
There was a problem hiding this comment.
Approved. Splitting #109 paid off: this one is focused and easy to follow. Moving every breakpoint into Puls3Breakpoints with no raw numbers left in lib, and keeping TopBar presentational while AppShell owns routing, are both nice touches.
None of these block the merge. They're worth picking up in #112 or a small follow-up:
Puls3Text.compactis a mutable global (puls3_theme.dart:126). It is set insidePuls3App.build(app.dart:85) and read byisDense/visualDensityand the text styles. Correctness then depends on build order, and the value can leak between widget tests in the same process. Since the theme is already rebuilt fromMediaQuery, I'd passcompactinto the theme builder, or read it fromcontextwhere it's needed.isCompactLayoutisn't used everywhere.app.dart:86andtop_bar.dart:50repeatwidth < Puls3Breakpoints.compactby hand, so changing the rule in one place would miss them. The "compact headline" rule is also duplicated in 4 places (screen_header,market_screen,studio_screen,landing_screen). APuls3Text.headline(context)would give it a single owner.- No tests at the thresholds. The tests cover 390 px and desktop, but not widths just below and above 640, 700, 900 or 1000, so a typo in a threshold passes CI. A small parameterized test around each boundary would cover it.
Smaller notes: the compact branch of _NavLink can't be reached because AppShell passes links: const [] on phones (top_bar.dart:66-73); on an unknown route the bottom bar selects Studio while the top bar selects nothing (app_shell.dart:36); and 600/640/700 are close enough that a one-line comment explaining why they differ would help.
Deploying puls3 with
|
| Latest commit: |
b10b8b9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://68228594.puls3-4lw.pages.dev |
| Branch Preview URL: | https://feat-flutter-theme-shell.puls3-4lw.pages.dev |
|
@Pericena merged as 1a860e8, thanks! Since it was squash-merged, #112 and #113 still carry the original theme/shell commit. Rebasing them onto |
Part A of the #109 split (theme and shell). No screen content changes.
Puls3Breakpoints: one set of named breakpoints instead of raw 600/640/700/900/960/1000 numbers, plusisCompactLayout().PrimaryButtonis 44 px there (52 on desktop) inside a 48 px tap target.ScreenHeaderandSkeletonBoxbuilding blocks for the next parts.Tests
Bottom bar on phones only, compact type and button size, no overflow on every screen at 320/360/390/414 px.
flutter analyze: 0 issues ·flutter test: 94/94