[Scribe mobile] Add and enable Scribe mobile#4245
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
WalkthroughExtends HtmlUtils selection APIs with an isWebPlatform flag, adds save/restore/clear selection scripts, and makes selection coordinate logic platform-aware (web vs mobile). Removes mobile-only guards so AIScribe config and bindings can run on mobile. Adds mobile-specific editor focus/save helpers, makes several composer and overlay methods async, wires an onOpenAiAssistantModal callback into multiple app-bar and bottom-bar widgets, and introduces mobile AIScribe UI widgets and modal bottom-sheet flows. Several new exports, image path, and style constants were also added. Possibly related PRs
Suggested labels
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 |
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
2d1cab2 to
a29acb2
Compare
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
a29acb2 to
a9a21c0
Compare
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4245. |
|
@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 (5)
scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart (1)
184-186: ConsiderEdgeInsetsDirectionalfor RTL consistency.The new
backIconPaddingusesEdgeInsets.only(right: ...)while other paddings in this file useEdgeInsetsDirectional. If the back button should have its padding adapt for RTL layouts, consider:♻️ Suggested change for RTL support
- static const EdgeInsetsGeometry backIconPadding = - EdgeInsets.only(right: 8.0, top: 8.0, bottom: 8.0); + static const EdgeInsetsGeometry backIconPadding = + EdgeInsetsDirectional.only(end: 8.0, top: 8.0, bottom: 8.0);If the back icon intentionally does not adapt for RTL, this can be ignored.
lib/features/composer/presentation/widgets/mobile/app_bar_composer_widget.dart (1)
55-65: Minor styling inconsistency withLandscapeAppBarComposerWidget.The AI assistant button here doesn't explicitly set
iconColor: MobileAppBarComposerWidgetStyle.iconColor, unlike the same button inLandscapeAppBarComposerWidget(line 62). For visual consistency across orientations, consider adding theiconColorproperty.Suggested fix
TMailButtonWidget.fromIcon( icon: imagePaths.icGradientSparkle, backgroundColor: Colors.transparent, + iconColor: MobileAppBarComposerWidgetStyle.iconColor, iconSize: MobileAppBarComposerWidgetStyle.iconSize, tooltipMessage: ScribeLocalizations.of(context).aiAssistant, onTapActionCallback: () => onOpenAiAssistantModal!(null, null), ),scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dart (1)
94-108: Remove unusedcontentparameter fromshowMobileAIScribeMenuModal.The
contentparameter is accepted but not passed toAiScribeMobileActionsBottomSheet, which doesn't accept it. UnlikeshowMobileAIScribeSuggestionModal, the menu modal only needs image paths and available categories. Remove the parameter to avoid confusion.scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart (2)
57-59: Consider using consistent icon sizing.The back chevron uses
AIScribeSizes.aiAssistantIcon(line 59) while the close button uses a hardcoded24(line 73). For consistency and maintainability, consider extracting the close icon size to a constant or reusing the existing size constant if appropriate.♻️ Suggested improvement
IconButton( icon: const Icon( Icons.close, - size: 24, + size: AIScribeSizes.aiAssistantIcon, ),Also applies to: 70-74
99-111: Optional: Extract to local variable for null-safety clarity.The force-unwrap on line 104 is safe because
itemBuilderonly runs whenitemCount > 0, but the reasoning is implicit. A local variable would make the null-safety more explicit.♻️ Suggested improvement
Widget _buildSubmenuListView() { + final submenuActions = _selectedCategory!.submenuActions ?? []; return ListView.builder( shrinkWrap: true, - itemCount: _selectedCategory!.submenuActions?.length ?? 0, + itemCount: submenuActions.length, itemBuilder: (context, index) { - final submenuAction = _selectedCategory!.submenuActions![index]; + final submenuAction = submenuActions[index]; return AiScribeSubmenuItem( menuAction: submenuAction, onSelectAction: _onActionSelected, ); }, ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
core/lib/utils/html/html_utils.dartlib/features/base/mixin/ai_scribe_mixin.dartlib/features/composer/presentation/composer_view.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/composer/presentation/widgets/mobile/app_bar_composer_widget.dartlib/features/composer/presentation/widgets/mobile/landscape_app_bar_composer_widget.dartlib/features/composer/presentation/widgets/mobile/mobile_editor_widget.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_cached_ai_scribe_extension.dartscribe/lib/scribe.dartscribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dartscribe/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.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_suggestion_bottom_sheet.dart
💤 Files with no reviewable changes (2)
- lib/features/base/mixin/ai_scribe_mixin.dart
- lib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_cached_ai_scribe_extension.dart
🧰 Additional context used
🧠 Learnings (3)
📚 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.dartlib/features/composer/presentation/widgets/mobile/landscape_app_bar_composer_widget.dartscribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartcore/lib/utils/html/html_utils.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dartscribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dartlib/features/composer/presentation/composer_view.dartlib/features/composer/presentation/widgets/mobile/mobile_editor_widget.dartscribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dartlib/features/composer/presentation/widgets/mobile/app_bar_composer_widget.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_suggestion_bottom_sheet.dart
📚 Learning: 2025-12-12T09:03:05.962Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/model/popup_menu_item_email_action.dart:34-35
Timestamp: 2025-12-12T09:03:05.962Z
Learning: In the tmail-flutter codebase, popup menu items intentionally use a consistent hoverIcon (imagePaths.icThumbsUp) across all popup item types (email actions, mailbox actions, profile settings, etc.), while actionIcon varies per action type. This is a deliberate UI/UX design choice.
Applied to files:
lib/features/composer/presentation/widgets/mobile/landscape_app_bar_composer_widget.dartscribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dartscribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dartlib/features/composer/presentation/widgets/mobile/app_bar_composer_widget.dart
📚 Learning: 2025-12-12T04:54:11.121Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4191
File: lib/features/email/presentation/extensions/handle_email_action_extension.dart:37-80
Timestamp: 2025-12-12T04:54:11.121Z
Learning: In lib/features/email/presentation/extensions/handle_email_action_extension.dart, the mailboxDashBoardController.selectedEmail should only be synchronized when isMobileThreadDisabled is true. This is intentional behavior and should not be changed to update selectedEmail in non-mobile or thread-enabled contexts.
Applied to files:
core/lib/utils/html/html_utils.dartlib/features/composer/presentation/widgets/mobile/mobile_editor_widget.dart
🔇 Additional comments (20)
lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart (1)
11-22: LGTM!The removal of the mobile platform restriction correctly enables AIScribe on mobile when both the config is enabled and the endpoint is available. The rest of the extension already handles platform differences appropriately via
PlatformInfo.isWebchecks.scribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dart (1)
77-98: Good fix for the missing grammar icon.Adding the
correctGrammarcase ensures the grammar action displays its icon consistently with the category. Previously, this would fall through to thedefaultcase and returnnull.lib/features/composer/presentation/widgets/mobile/mobile_editor_widget.dart (1)
94-101: LGTM!Good fix for the iOS AI Scribe overlay button display issue. Delaying the selection listener setup to
onCompletedensures the WebView is fully loaded before registering the JavaScript handler, preventing race conditions on iOS.core/lib/utils/html/html_utils.dart (3)
82-83: LGTM on platform detection.The
isDesktopEditorcheck using!window.flutter_inappwebviewcorrectly distinguishes between web iframe (desktop) and InAppWebView (mobile) contexts.
103-117: LGTM on platform-specific selector logic.The selector branching correctly targets:
- Desktop:
.note-editor .note-editable(Summernote editor)- Mobile:
#editor(InAppWebView HTML editor)
154-160: Verify the mobile offset values on various devices.The arbitrary offset
{ x: 22, y: -20 }to avoid native selection marks may need adjustment depending on device densities or OS versions. Consider documenting or making these values configurable if issues arise during testing.scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_suggestion_bottom_sheet.dart (2)
5-68: Well-structured mobile suggestion bottom sheet implementation.The widget correctly implements
AiScribeSuggestionStateMixinwith proper delegation to widget properties. The use ofFlexiblefor the content area allows the state content to expand within the full-height container while respecting the header's space.
70-102: LGTM!The state overrides follow a clean pattern: wrapping the base mixin implementations with consistent padding. The error state's vertical centering provides better UX for mobile screens.
scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dart (2)
20-44: Clean platform-aware branching for menu modal.The branching logic correctly separates mobile and desktop paths. The desktop path properly manages the
PopupSubmenuControllerlifecycle withwhenComplete(submenuController.dispose).
110-128: LGTM!The mobile suggestion modal implementation correctly passes all required parameters including
contentandonSelectAiScribeSuggestionAction. TheisScrollControlled: trueanduseSafeArea: truesettings are appropriate for a full-screen modal experience.scribe/lib/scribe.dart (1)
46-48: LGTM!The new mobile widget exports follow the established pattern and correctly expose the new mobile AI Scribe UI components for external consumption.
lib/features/composer/presentation/widgets/mobile/landscape_app_bar_composer_widget.dart (2)
57-68: LGTM!The AI assistant button integration follows the existing widget patterns with proper conditional rendering, consistent styling, and localized tooltip. The placement after
Spacer()ensures it appears in the action buttons area.
4-5: Theai_assistant_button.dartimport is not unused. TheOnOpenAiAssistantModaltypedef from this import is actively used in line 21 as the type for theonOpenAiAssistantModalfield.Likely an incorrect or invalid review comment.
lib/features/composer/presentation/composer_view.dart (2)
69-71: LGTM!The conditional wiring
controller.isAIScribeAvailable ? controller.openAIAssistantModal : nullcorrectly enables the AI assistant button only when the feature is available.
93-95: Consistent AI assistant modal wiring across layouts.The
onOpenAiAssistantModalcallback is wired identically for both portrait (AppBarComposerWidget) and tablet (TabletBottomBarComposerWidget) layouts, ensuring consistent behavior across all mobile form factors.Also applies to: 479-481
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart (2)
19-35: LGTM!The category handling correctly routes to
onCategorySelectedfor category items (allowing submenu navigation) andonActionSelectedfor direct actions. The type check ensures proper callback routing.
37-53: Handle or clarify thesubmenuActions.length > 1case.When
submenuActions.length > 1, the code falls through to render a plainAiScribeMenuItemwithout explicitly showing all submenu actions. While this case may be data-impossible given the model constraints, either add explicit handling (e.g., a comment explaining why this is safe) or handle it like the bottom sheet does by iterating all actions.scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart (3)
122-134: Good keyboard-aware padding implementation.The use of
MediaQuery.of(context).viewInsets.bottomproperly handles keyboard appearance, ensuring the input bar remains accessible when the keyboard is visible.
139-179: LGTM - Well-structured full-screen modal layout.The build method properly:
- Uses
SafeAreato handle device notches and home indicators- Organizes content with appropriate flex behavior
- Maintains a sticky bottom bar for custom prompts
The
height: double.infinityaligns with the PR objective of implementing a full-screen modal.
25-29: The silent no-op for non-AiScribeActionContextMenuActiontypes is intentional design.The code uses type-based routing:
AiScribeMobileActionsItemfiltersAiScribeCategoryContextMenuActioninstances and routes them toonCategorySelected, while all other action types flow toonActionSelected. This pattern is consistent throughout the file—different action types are handled by separate callbacks (onCategorySelected,onActionSelected,onCustomPromptSubmit). The_onActionSelectedmethod correctly handlesAiScribeActionContextMenuActionas the primary type it receives.
|
lib/features/composer/presentation/widgets/mobile/app_bar_composer_widget.dart
Outdated
Show resolved
Hide resolved
lib/features/composer/presentation/widgets/mobile/landscape_app_bar_composer_widget.dart
Outdated
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dart
Outdated
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dart
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dart
Outdated
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_suggestion_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_suggestion_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_suggestion_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
a9a21c0 to
ce3632f
Compare
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
ce3632f to
7ed5cd8
Compare
This is because we miss linagora/enough_html_editor#40. Can we merge it? |
We've merged it, please update it in the pubspec so we can retest it. @zatteo |
Done 👍 |
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
45e271f to
d025cae
Compare
Done 👍 |
|
Because the HTML editor is different, we need a different selector to get the editable element. We also need to add a small offset to the coordinates to avoid positionning the overlay icon above native selection UI in mobile. For example, Android adds a marker at the beginning and the end of the selection and we do not want to display the Scribe overlay icon above this marker.
The button was never displayed because the selectionchange event listener was loaded when the WebView was not entirely loaded so selectionchange event were never fired. Now we setup selection listener when the WebView is loaded instead of created.
- Same icon than for desktop - Displayed only if AI Scribe is enabled
Same features than Scribe desktop but as a full screen modal instead of a context menu. Reuse menu and submenu components. Reuse suggestion components. Reuse 99% of Scribe desktop style.
When opening Scribe mobile modal, we now unfocus the composer to hide the keyboard and the selection start and end icons from the OS. Add new methods to save, restore, and clear text selection in HTML editor. These methods allow preserving and restoring user selection state when opening the Scribe mobile modal so that the "replace" action still works.
enough_html_editor uses execCommand to insert HTML. However execCommand only works if the contenteditable element is focused. So inserting or replacing Scribe result do not work if the user did not click before in the composer. Here we manually focus the contenteditable element to fix this before inserting content.
- Fix Scribe replace on mobile when opened from menu bar (to be complete this fix needs this PR Enough-Software/enough_html_editor#37) - Ensure all mobile editor call are awaited - Do not clear text if a selection has been restored (which mean we replace a selection) - Avoid collapseToEnd crash
- Add getSavedSelection to check saved selection without consuming it - Distinguish between setText (replace all) and insertText (at cursor) operations - Fix replace callback to use setText when no selection exists - Clean up saved selection state after restoration in HTML utils - Move mobile editor focus and selection restore to appropriate callbacks
In recent commits I separated how we could add text in the editor. So we need to escape HTML in both method that add text: insertTextInEditor and setTextInEditor. I had to extract the HTML escape part of convertTextContentToHtmlContent because when using setTextInEditor we do not need to convert \n to <br>.
To enjoy the fix from the following PR linagora/enough_html_editor#40
zatteo
force-pushed
the
feat/scribe-mobile
branch
from
d025cae to
41f6580
Compare
4 participants
Before merging the Scribe desktop PR, we left the scribe as is (unfinished) after disabling it.
Here I:
See PR #4247 for the keyboard and selection visible in the modal fix.
Screen.Recording.2026-01-08.at.17.23.01.mov
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.