feat(assets): real walkthrough illustrations and Mostro logo across app bars - #264
Conversation
Replace psychology icon with Mostro logo in chat app bar, center logo in home screen app bar using Stack layout, and standardize logo height to 32px across all screens. Remove unused green color parameter from ChatAppBar.
…size Replace all PNG logo and walkthrough images with WebP equivalents across app bars, drawer, and walkthrough screens. Add test coverage to verify WebP assets decode correctly.
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdded WebP logo and walkthrough assets. Updated walkthrough references and documentation. Replaced several app-bar and drawer branding elements with Mostro logo assets. ChangesWebP asset migration
Branding surface updates
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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: 433a2a7e63
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
lib/features/home/screens/home_screen.dart (1)
315-318: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse one localized semantic label for both app-bar logos.
If these images replace the previous title semantics, add a dedicated ARB label and pass it to both images. Flutter provides
Image.semanticLabelfor assistive technologies. (api.flutter.dev)
lib/features/home/screens/home_screen.dart#L315-L318: Pass the localized label to the centered logo.lib/features/trades/screens/trades_screen.dart#L139-L141: Pass the same localized label to the AppBar title image.As per coding guidelines, user-facing strings in Dart must come from
AppLocalizations.of(context)rather than hard-coded literals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/home/screens/home_screen.dart` around lines 315 - 318, Define one localized ARB semantic label and retrieve it through AppLocalizations.of(context); pass that label via Image.semanticLabel to the centered logo in home_screen.dart (315-318) and the AppBar title logo in trades_screen.dart (139-141), ensuring both images use the same localized label.Source: Coding guidelines
test/tmp_webp_test.dart (2)
7-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the permanent test and place it in the matching test subtree.
test/tmp_webp_test.dartis permanent asset coverage, buttmpmakes it look like a disposable debug test. Move it to a stable asset-focused*_test.dartpath that matches the covered feature or asset area.As per coding guidelines,
test/**/*_test.dartrequires: “Place Dart tests undertest/, mirror the feature layout, and use the*_test.dartsuffix.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/tmp_webp_test.dart` around lines 7 - 21, Rename the permanent WebP asset coverage test from tmp_webp_test.dart to a stable asset-focused *_test.dart location under the matching test subtree. Preserve the existing _assets list and “every webp asset decodes” test behavior while ensuring the new path mirrors the covered asset or feature area and no longer suggests a temporary debug test.Source: Coding guidelines
24-37: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDispose the decoded image resources.
This test creates a
ui.Codecandui.Imagefor each asset but never disposes them. Wrap the frame decoding and assertions infinallyblocks soframe.image.dispose()andcodec.dispose()run even if expectations fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/tmp_webp_test.dart` around lines 24 - 37, Update the asset loop’s image decoding logic to dispose resources reliably: wrap the codec/frame processing and assertions in a finally block that disposes frame.image, and ensure codec.dispose() also runs when decoding or expectations fail. Preserve the existing logging and dimension assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/features/home/screens/home_screen.dart`:
- Around line 315-318: Define one localized ARB semantic label and retrieve it
through AppLocalizations.of(context); pass that label via Image.semanticLabel to
the centered logo in home_screen.dart (315-318) and the AppBar title logo in
trades_screen.dart (139-141), ensuring both images use the same localized label.
In `@test/tmp_webp_test.dart`:
- Around line 7-21: Rename the permanent WebP asset coverage test from
tmp_webp_test.dart to a stable asset-focused *_test.dart location under the
matching test subtree. Preserve the existing _assets list and “every webp asset
decodes” test behavior while ensuring the new path mirrors the covered asset or
feature area and no longer suggests a temporary debug test.
- Around line 24-37: Update the asset loop’s image decoding logic to dispose
resources reliably: wrap the codec/frame processing and assertions in a finally
block that disposes frame.image, and ensure codec.dispose() also runs when
decoding or expectations fail. Preserve the existing logging and dimension
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d32f764d-619e-4c46-a70a-07909c752b5e
⛔ Files ignored due to path filters (6)
assets/images/wt-1.pngis excluded by!**/*.pngassets/images/wt-2.pngis excluded by!**/*.pngassets/images/wt-3.pngis excluded by!**/*.pngassets/images/wt-4.pngis excluded by!**/*.pngassets/images/wt-5.pngis excluded by!**/*.pngassets/images/wt-6.pngis excluded by!**/*.png
📒 Files selected for processing (14)
assets/images/mostro_logo.webpassets/images/mostro_logo_beta.webpassets/images/wt-1.webpassets/images/wt-2.webpassets/images/wt-3.webpassets/images/wt-4.webpassets/images/wt-5.webpassets/images/wt-6.webplib/features/chat/screens/chat_rooms_screen.dartlib/features/drawer/screens/drawer_menu.dartlib/features/home/screens/home_screen.dartlib/features/trades/screens/trades_screen.dartlib/features/walkthrough/screens/walkthrough_screen.darttest/tmp_webp_test.dart
Review round 1 — dispositionsCodeRabbit — CodeRabbit — Codex — three P1s |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/004-mostro-p2p-client/plan.md`:
- Line 142: Synchronize the walkthrough asset names documented in the task entry
with the existing WebP contract used by the plan and WalkthroughScreen, updating
wt-1 through wt-6 to .webp or explicitly marking the PNG names as historical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e5b7fb6f-6f21-4a15-83e1-f0b4950e8a3e
📒 Files selected for processing (1)
specs/004-mostro-p2p-client/plan.md
2 participants
Replaces the placeholder walkthrough assets with the real v1 illustrations and wires the Mostro logo into every app bar.
wt-1..6were 817-byte flat rectangles in the app background colour; now the actual illustrations fromMostroP2P/mobile(1:1 slide mapping, the six slide texts are identical in both repos).Icons.psychologyplaceholder). Drawer header uses the beta logo image instead of the icon + text + chip composition.assets/images/drops from 2.6 MB to 168 KB (-94%). RMSE vs the originals is 0.9-1.4% on the illustrations and 0 on the logo; alpha is preserved.Closes #263
Summary by CodeRabbit
New Features
Improvements
Documentation