-
Notifications
You must be signed in to change notification settings - Fork 520
Align mobile Flutter branding with PWA (if possible) #626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add AppConfig class with product_name and brand_name following Rails pattern (configurable via --dart-define at build time) - Create design system (SureColors, SureBorderRadius, SureShadows, SureTypography) matching the Rails/Tailwind design tokens - Add SureTheme builder for light/dark themes using design system - Update main.dart to use AppConfig and SureTheme - Update example URLs to use app.sure.am
📝 WalkthroughWalkthroughCentralizes version reading via a repository Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/cc @dwvwdv FYI |
- Add shared VERSION file (0.6.7-alpha.8) at repo root - Update Rails version.rb to read from VERSION file - Update Flutter pubspec.yaml to match (0.6.7-alpha.8+8) - Add bin/sync-mobile-version script to sync versions The sync script can be run before Flutter builds or in CI: bin/sync-mobile-version # Update pubspec.yaml bin/sync-mobile-version --check # Verify versions match
| static String get appTitle => '$productName Finance'; | ||
|
|
||
| /// Copyright notice with brand name. | ||
| static String get copyrightNotice => '\u00A9 ${DateTime.now().year} $brandName'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where should this content be displayed? Perhaps it would be more appropriate to place it at the bottom of the "Settings" page?
look goods. You could also add a theme switching menu to the settings page for easier addition of themes in the future. |
Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@config/initializers/version.rb`:
- Around line 17-23: The fallback version string currently returns
"0.6.7-alpha.9" when version_file (the Rails.root.join("VERSION") read block) is
missing; change that fallback to match or predate the actual VERSION file value
(e.g., "0.6.7-alpha.8") so the app never reports a newer release than exists, or
if the higher fallback is intentional, add a brief comment above the conditional
explaining why the fallback is intentionally newer for development/testing.
🧹 Nitpick comments (1)
mobile/lib/theme/sure_theme.dart (1)
289-383:SureTypography.fontFamilySansis not applied to the text theme.The design system defines
Geistas the primary font family inSureTypography, but_buildTextThemedoesn't setfontFamilyon anyTextStyle. This means the app will use the default system font instead of Geist.♻️ Suggested fix
static TextTheme _buildTextTheme(Brightness brightness) { final Color primaryTextColor = brightness == Brightness.dark ? SureColors.white : SureColors.gray900; final Color secondaryTextColor = brightness == Brightness.dark ? SureColors.gray300 : SureColors.gray500; return TextTheme( displayLarge: TextStyle( fontSize: 57, fontWeight: FontWeight.w400, color: primaryTextColor, letterSpacing: -0.25, + fontFamily: SureTypography.fontFamilySans, + fontFamilyFallback: SureTypography.fontFamilySansFallback, ), // ... apply to other text stylesAlternatively, apply the font family at the
ThemeDatalevel:return ThemeData( fontFamily: SureTypography.fontFamilySans, fontFamilyFallback: SureTypography.fontFamilySansFallback, // ... );Ensure the Geist font assets are bundled in
pubspec.yamlunder thefontssection for this to work.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
VERSIONbin/sync-mobile-versionconfig/initializers/version.rbmobile/lib/config/app_config.dartmobile/lib/main.dartmobile/lib/screens/backend_config_screen.dartmobile/lib/theme/design_system.dartmobile/lib/theme/sure_theme.dartmobile/pubspec.yaml
🧰 Additional context used
📓 Path-based instructions (6)
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use 2-space indentation for Ruby code
Usesnake_casefor method and variable names in Ruby
UseCamelCasefor class and module names in Ruby
Runbin/rubocopbefore pushing code; use-Aflag to auto-correct safe style violations
**/*.rb: UseCurrent.userfor the current user. Do NOT usecurrent_user.
UseCurrent.familyfor the current family. Do NOT usecurrent_family.
Files:
config/initializers/version.rb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
config/initializers/version.rb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
config/initializers/version.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
config/initializers/version.rb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Place configuration files in
config/directory
Files:
config/initializers/version.rb
**/*.{erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,rb}: All user-facing strings must use localization (i18n). Always uset()helper. Update locale files for each new or changed element.
Use i18n interpolation for dynamic content:t("users.greeting", name: user.name)
Use Rails pluralization in i18n:t("transactions.count", count:@transactions.count)
Files:
config/initializers/version.rb
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,js,ts,tsx} : Use functional tokens defined in design system (e.g., `text-primary` instead of `text-white`, `bg-container` instead of `bg-white`)
📚 Learning: 2026-01-02T09:16:53.865Z
Learnt from: dwvwdv
Repo: we-promise/sure PR: 426
File: mobile/lib/providers/accounts_provider.dart:97-103
Timestamp: 2026-01-02T09:16:53.865Z
Learning: In the Flutter mobile app (mobile/ directory), the maintainer prefers keeping error messages simple and user-friendly rather than exposing detailed technical error information, even when catching generic exceptions in provider classes.
Applied to files:
mobile/lib/main.dart
📚 Learning: 2026-01-09T12:25:59.839Z
Learnt from: jjmata
Repo: we-promise/sure PR: 593
File: app/controllers/pages_controller.rb:200-203
Timestamp: 2026-01-09T12:25:59.839Z
Learning: Do not compare hard-coded strings to identify the 'Uncategorized Investments' category in Ruby/Rails code (e.g., ct.category.name != "Uncategorized Investments"). Localization/internationalization can change strings. Prefer: (1) a model-level predicate such as ct.category.uncategorized_investment? or (2) compare by identity with a canonical category, e.g., ct.category != Category.uncategorized_investments. This improves maintainability and correctness across translations. Apply this pattern to Ruby files that perform category checks (controllers, models, or elsewhere).
Applied to files:
config/initializers/version.rb
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,js,ts,tsx} : Use functional tokens defined in design system (e.g., `text-primary` instead of `text-white`, `bg-container` instead of `bg-white`)
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2025-11-24T16:57:09.166Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:57:09.166Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Always reference app/assets/tailwind/maybe-design-system.css for design system tokens and primitives
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2025-08-22T18:30:26.758Z
Learnt from: jjmata
Repo: we-promise/sure PR: 116
File: app/views/family_exports/_list.html.erb:16-28
Timestamp: 2025-08-22T18:30:26.758Z
Learning: In the family exports UI (app/views/family_exports/_list.html.erb), raw color utilities like bg-green-500/5, text-green-500, bg-red-500/5, text-red-500 are preferred over design system tokens for status indicators to maintain UI legibility and consistency with the existing import UI patterns. The user has indicated that design token replacements would make the UI illegible in this context.
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to app/assets/tailwind/maybe-design-system.css : NEVER create new styles in design system files without permission
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/views/**/*.erb : Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,js,ts,tsx} : In TailwindCSS, always reference `app/assets/tailwind/maybe-design-system.css` for primitives and tokens
Applied to files:
mobile/lib/theme/design_system.dart
📚 Learning: 2025-11-24T16:57:09.166Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:57:09.166Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Applied to files:
mobile/lib/theme/design_system.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: ci / test
- GitHub Check: Build Android APK
- GitHub Check: Build iOS IPA
🔇 Additional comments (13)
VERSION (1)
1-1: LGTM!Single source of truth for versioning across the monorepo. The format aligns with semver expectations in the consuming scripts (
config/initializers/version.rbandbin/sync-mobile-version).mobile/pubspec.yaml (1)
4-4: LGTM!Version correctly aligned with the monorepo
VERSIONfile (0.6.7-alpha.8) using Flutter's format with build number derived from the prerelease suffix.mobile/lib/screens/backend_config_screen.dart (1)
228-228: LGTM!Consistent URL branding update to
https://app.sure.amin both the example list and hint text.Also applies to: 321-321
bin/sync-mobile-version (1)
1-102: LGTM!Well-structured synchronization script with:
- Clear usage documentation
- Proper error handling with informative messages
- CI-friendly
--checkmode with appropriate exit codes- Correct semver parsing and Flutter version format conversion
mobile/lib/theme/design_system.dart (3)
193-305: Well-structured semantic color system.The context-aware color helpers properly map light/dark mode variants and align with the functional token pattern from the Rails design system. The use of
withValues(alpha:)is the correct modern Flutter API.
314-420: LGTM!Typography, border radius, and shadow utilities are well-implemented with proper font fallbacks and context-aware shadow colors for light/dark mode.
98-99: Design system bug in Rails:cyan-800andcyan-900are identical.The duplicate
0xFF155B75incyan800andcyan900exists in both the Flutter code AND the authoritative Rails design system (app/assets/tailwind/maybe-design-system.css). This is the only color scale with this issue—all other scales (blue, fuchsia, gray, green, indigo, orange, pink, red, yellow) have properly distinct 800 and 900 values.The Flutter implementation is correct. The bug is in the Rails design system file, which should be fixed first; then the Flutter values can be updated to match.
Likely an incorrect or invalid review comment.
mobile/lib/theme/sure_theme.dart (2)
14-149: Comprehensive light theme configuration.The theme properly uses design system tokens for all component themes, ensuring visual consistency with the Rails/Tailwind design system. Good use of
ColorScheme.fromSeedwith explicit overrides.
151-287: Dark theme mirrors light theme structure correctly.Proper dark mode color mappings using the appropriate design system tokens (e.g.,
gray900for surfaces,gray700for borders).mobile/lib/main.dart (3)
3-3: Clean import additions for config and theme.The new imports properly integrate the
AppConfigandSureThememodules.Also applies to: 14-14
21-21: Dynamic product name in startup log.The log message now uses
AppConfig.productName, supporting compile-time branding customization.
65-69: MaterialApp properly configured with new theme and config.The app title and themes are now sourced from centralized configuration, enabling consistent branding across builds.
mobile/lib/config/app_config.dart (1)
14-36: Well-designed compile-time configuration class.The use of
String.fromEnvironmentwith defaults correctly mirrors the Rails pattern and enables build-time customization via--dart-define. The private constructor appropriately prevents instantiation of this utility class.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Read from shared VERSION file at repo root | ||
| version_file = Rails.root.join("VERSION") | ||
| if version_file.exist? | ||
| version_file.read.strip | ||
| else | ||
| "0.6.7-alpha.9" | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback version is higher than the current VERSION file value.
The VERSION file contains 0.6.7-alpha.8, but the fallback is 0.6.7-alpha.9. If the VERSION file is missing during deployment, the app would incorrectly report a newer version than the actual release.
Consider aligning the fallback to match or predate the current version:
Suggested fix
if version_file.exist?
version_file.read.strip
else
- "0.6.7-alpha.9"
+ "0.6.7-alpha.8"
endAlternatively, if the fallback is intentional for development purposes, add a comment explaining the rationale.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Read from shared VERSION file at repo root | |
| version_file = Rails.root.join("VERSION") | |
| if version_file.exist? | |
| version_file.read.strip | |
| else | |
| "0.6.7-alpha.9" | |
| end | |
| # Read from shared VERSION file at repo root | |
| version_file = Rails.root.join("VERSION") | |
| if version_file.exist? | |
| version_file.read.strip | |
| else | |
| "0.6.7-alpha.8" | |
| end |
🤖 Prompt for AI Agents
In `@config/initializers/version.rb` around lines 17 - 23, The fallback version
string currently returns "0.6.7-alpha.9" when version_file (the
Rails.root.join("VERSION") read block) is missing; change that fallback to match
or predate the actual VERSION file value (e.g., "0.6.7-alpha.8") so the app
never reports a newer release than exists, or if the higher fallback is
intentional, add a brief comment above the conditional explaining why the
fallback is intentionally newer for development/testing.
Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
|
@jjmata Is something missing here? Why isn't it merged? I'm planning to adjust some styles recently, and making changes now might cause some conflicts. |
|
Nothing missing, just me wondering if these are "best practices" or not. 🤷♂️ What do you think? Seems a little Quixotic to try to keep uniformity of design system between PWA and Flutter, no? Don't let this block your changes, I can always rebase mine. |
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.