[Scribe mobile] Add current text in a card in Scribe mobile#4248
[Scribe mobile] Add current text in a card in Scribe mobile#4248hoangdat merged 1 commit intoscribe-mobilefrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis pull request extends the AIScribe presentation layer by introducing new styling constants and adding content display functionality. Changes include: new shadow, text, and sizing style definitions for a contentCard component in the styles module; propagation of an optional content parameter through the mobile modal manager to the mobile bottom sheet widget; and implementation of a private text card widget builder to render and display this content within the bottom sheet UI. No changes to core action or navigation logic were made. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart (2)
47-66: First shadow in list has no visual effect.The first
BoxShadow(lines 48-53) withspreadRadius: 0,blurRadius: 0, andoffset: Offset(0, 0)renders nothing visible. If this is intentional for layering or future use, consider adding a comment; otherwise, it can be removed.♻️ Suggested simplification
static final List<BoxShadow> contentCard = [ - BoxShadow( - color: AppColor.gray424244.withValues(alpha: 0.12), - spreadRadius: 0, - blurRadius: 0, - offset: const Offset(0, 0), - ), BoxShadow( color: Colors.black.withValues(alpha: 0.1), spreadRadius: 0, blurRadius: 3, offset: const Offset(0, 1), ), BoxShadow( color: Colors.black.withValues(alpha: 0.1), spreadRadius: 0, blurRadius: 2, offset: const Offset(0, 2), ), ];
116-122: Consider reusing existingsuggestionContentstyle.The
contentCardstyle is identical tosuggestionContent(lines 108-114). If they should stay in sync, consider reusing:static final TextStyle contentCard = suggestionContent;If they may diverge in the future, keeping them separate is fine.
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart (1)
128-153: UseAIScribeColors.backgroundfor consistency.Line 141 uses hardcoded
Colors.whiteinstead of the definedAIScribeColors.backgroundconstant. This ensures the color stays consistent if the theme changes.♻️ Suggested fix
decoration: BoxDecoration( - color: Colors.white, + color: AIScribeColors.background, borderRadius: BorderRadius.circular(AIScribeSizes.contentCardRadius), boxShadow: AIScribeShadows.contentCard, ),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dartscribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-09T09:36:45.349Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4194
File: lib/features/manage_account/presentation/manage_account_dashboard_controller.dart:174-176
Timestamp: 2025-12-09T09:36:45.349Z
Learning: In Dart/Flutter projects using GetX, do not wrap getBinding<T>() calls in try/catch since they return null when not found. Only wrap Get.find<T>() calls in try/catch because they throw if a dependency is unavailable. When a binding is optional, consider checking Get.isRegistered<T>() or handling the null/exception path gracefully instead of blindly catching, and document the expectation for failure modes where a dependency may not be registered.
Applied to files:
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dartscribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dartscribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.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). (10)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (forward)
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: Build web version and deploy
🔇 Additional comments (5)
scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart (1)
137-137: LGTM!The new sizing and spacing constants (
contentCardRadius,contentCardMaxHeight,contentCardMargin,contentCardPadding) follow existing naming conventions and patterns in the file.Also applies to: 155-155, 218-223
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart (2)
6-16: LGTM!The optional
contentparameter is properly declared and integrated into the widget constructor.
206-218: LGTM!The text card is correctly positioned between the header and the menu list, which aligns with the PR objective of showing content above the AI actions.
scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dart (2)
94-112: LGTM!The
contentparameter is correctly added toshowMobileAIScribeMenuModaland properly forwarded toAiScribeMobileActionsBottomSheet.
22-27: LGTM!The
contentparameter is consistently propagated to both mobile and desktop modal paths.
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4248. |
zatteo
force-pushed
the
feat/scribe-mobile-content-card
branch
from
e46d791 to
7dd3f72
Compare
|
Nitpick from coderabbit fixed (one kept as is because it makes more sense to keep contentCard and suggestionContent style separated). |
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
7ed5cd8 to
c27fe98
Compare
zatteo
changed the title
zatteo
force-pushed
the
feat/scribe-mobile-content-card
branch
from
7dd3f72 to
8aa0343
Compare
zatteo
force-pushed
the
feat/scribe-mobile
branch
2 times, most recently
from
8fe0326 to
fb2bf94
Compare
|
Please rebase it |
zatteo
force-pushed
the
feat/scribe-mobile-content-card
branch
from
8aa0343 to
df3ed03
Compare
|
Actually it is already scrollable even if it is not explicitly shown. Do you think we should make it more explicit? Screen.Recording.2026-01-20.at.14.35.22.mov |
zatteo
force-pushed
the
feat/scribe-mobile
branch
2 times, most recently
from
d025cae to
41f6580
Compare
Because the Scribe mobile is a full screen modal, we do not see the current content (so either the selected text or entire composer text) when selecting our AI actions. That's why we add here a scrollable card that displays the current content.
zatteo
force-pushed
the
feat/scribe-mobile-content-card
branch
from
df3ed03 to
26abdaf
Compare
3 participants
Context
Because the Scribe mobile is a full screen modal, we do not see the current content (so either the selected text or entire composer text) when selecting our AI actions.
Done
That's why we add here a scrollable card that displays the current content (in green in screenshot).
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.