Migrate scribe-mobile branch to master branch#4382
Open
Conversation
In Scribe desktop, the suggestion is displayed in a popup. In Scribe mobile, it will be displayed in a full screen modal. But it share the same logic about loading, success and error. So here I extract this logic and use it in the AI Scribe desktop to be able to use it later in AI Scribe mobile.
Scribe is always composed of menu (aka category) and submenu (aka action) : - submenu item UI was already in an UI only component. I moved it to items folder. - menu item UI was mixed in a component with desktop related UX. So I extract the UI to create an UI only component. I moved it also to items folder.
We will need a custom boxShadow for mobile version.
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
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.
Previosuly, we display Scribe mobile (bottomsheet) if platform was mobile, so also Android or iOS tablet. Now we choose to display Scribe mobile relatively to display screens instead of platform to display the desktop Scribe (context menu) for tablets.
- Support tap in addition to hover for context menu - Improve clamping for greater screen sizes to avoid Scribe getting off screen - Set a padding to 0 to avoid random padding depending on screen sizes
- Button was not added in MobileResponsiveAppBar - Mobile behavior related scripts were not correctly added for web composer - Mobile behavior related scripts return value was not properly handled for web composer - In responsive web, bottomsheet modal clicks were intercepted by iframe behind
It has been forgotten when enabling AI Scribe in mobile.
- The context.mounted check was placed after onTapFallback?.call() - The onTapFallback function calls controller.saveAndUnfocusForModal() which unfocuses the editor - This caused the context to become unmounted so the context.mounted check would return early - As a result the modal would never be show
Fix Scribe Bar hint color in mobile Fix Scribe Bar spacing allocation in mobile USe borderRadius instead of ClipRRect
- Decided to use mobile UI in mobile landscape - Make mobile bottom sheet content scrollable except header and bottombar: bottom sheet will render correctly even if keyboard is opened and there is not a lot of space. It will be less usable than in portrait but all actions available.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds end-to-end AI Scribe and Linagora ecosystem features: new prompts asset and PromptService with prompt models, migrates AI APIs to message-based AIMessage lists and updates generateMessage signatures, and adds saved-selection scripts plus isWebPlatform support in HtmlUtils. Introduces many Scribe UI components and modal manager changes (mobile/desktop split, mobile bottom sheets, suggestion tooling, localization strings). Composer and editor code wired to open the AI assistant (focus/save/restore selection), adds RichTextMobileTabletController.focus, small core utilities (escapeTextContent, new image getter), and LinagoraEcosystem repository, interactor, and controller caching. Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
|
dab246
changed the title
scribe-mobile branch to master branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart (1)
118-188:⚠️ Potential issue | 🟠 MajorUse the resolved anchored layout on mobile too.
Line 148 pins the popup to
anchorPos.dy, and Lines 156-159 size it only from the space to the anchor’s right. For the new right-aligned AI entry points, that width goes negative near the screen edge, and the popup also overlaps the trigger instead of honoring the calculated placement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart` around lines 118 - 188, In _buildAnchoredLayout: the mobile branch (_isMobileView) currently pins top to anchorPos.dy and computes width from anchorPos.dx which causes negative widths and overlap for right-aligned entry points; instead use the resolved AnchoredModalLayoutCalculator result (layout) for positioning and sizing on mobile as well — set top = layout.top (or top/bottom from layout as available), bottom = layout.bottom if provided, height = layout.availableHeight, and compute width from the layout's left/right values or modalWidth clamped to available space (e.g., use layout.left and screen width or any layout-provided availableWidth) so the popup honors the calculated placement; update references in _buildAnchoredLayout to use layout.* fields rather than anchorPos.* for the mobile branch.
🧹 Nitpick comments (9)
lib/features/composer/presentation/controller/rich_text_mobile_tablet_controller.dart (1)
24-27: Guard the editor element before callingfocus()to avoid avoidable JS exceptions.On Line 26,
document.getElementById('editor').focus()can throw when the editor node is not yet available. You already catch it in Dart, but this can create noisy warnings during normal lifecycle timing.Proposed change
- await htmlEditorApi?.webViewController.evaluateJavascript(source: ''' - (() => { - document.getElementById('editor').focus(); - })();'''); + await htmlEditorApi?.webViewController.evaluateJavascript(source: ''' + (() => { + const editor = document.getElementById('editor'); + if (editor && typeof editor.focus === 'function') { + editor.focus(); + } + })();''');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/composer/presentation/controller/rich_text_mobile_tablet_controller.dart` around lines 24 - 27, The injected JS that calls document.getElementById('editor').focus() should first guard that the element exists to avoid transient JS exceptions; update the evaluateJavascript call (the call on htmlEditorApi?.webViewController.evaluateJavascript) to run a small IIFE that queries document.getElementById('editor') into a variable and only calls .focus() when that variable is non-null (e.g., if (el) el.focus()), so the Dart-side catch no longer sees noisy timing warnings.core/lib/utils/html/html_utils.dart (1)
155-161: Consider extracting magic numbers to named constants.The offset values
24and-24are documented as "arbitrary determined to avoid selection marks", which is helpful context. For maintainability, consider extracting these to named constants at the top of the script or as parameters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/lib/utils/html/html_utils.dart` around lines 155 - 161, The hard-coded offsets { x: 24, y: -24 } used when computing buttonOffset in the block that sets x/y (referencing isWebPlatform, buttonOffset, lastRect, editableRect) should be extracted to named constants (e.g., MOBILE_BUTTON_OFFSET_X and MOBILE_BUTTON_OFFSET_Y or a single MOBILE_BUTTON_OFFSET constant) declared near the top of the file or passed as parameters; replace the literals with those constants, add a short comment describing why they exist ("arbitrary values to avoid selection marks on Android/iOS"), and ensure any tests or callers that rely on this behavior are updated to use the new constants or parameters.scribe/lib/scribe/ai/presentation/utils/ai_scribe_mobile_utils.dart (1)
6-8: Cache theResponsiveUtilslookup to avoid redundantGet.findcalls.
Get.find<ResponsiveUtils>()is called twice in the same expression. Consider caching the result for cleaner code and to avoid redundant lookups.Additionally, based on learnings,
Get.find<T>()throws if the dependency is unavailable. IfResponsiveUtilsmight not be registered in some code paths, consider wrapping this in a try/catch or checkingGet.isRegistered<ResponsiveUtils>().♻️ Suggested refactor
class AiScribeMobileUtils { static bool isScribeInMobileMode(BuildContext? context) { - return context != null && (Get.find<ResponsiveUtils>().isMobile(context) || Get.find<ResponsiveUtils>().isLandscapeMobile(context)); + if (context == null) return false; + final responsiveUtils = Get.find<ResponsiveUtils>(); + return responsiveUtils.isMobile(context) || + responsiveUtils.isLandscapeMobile(context); } }Based on learnings: "In Dart/Flutter projects using GetX, [...] Only wrap Get.find() calls in try/catch because they throw if a dependency is unavailable."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/utils/ai_scribe_mobile_utils.dart` around lines 6 - 8, The method isScribeInMobileMode calls Get.find<ResponsiveUtils>() twice; cache the lookup by retrieving the ResponsiveUtils instance once (e.g., assign to a local variable inside isScribeInMobileMode) and use that for both isMobile and isLandscapeMobile checks, and guard the lookup with Get.isRegistered<ResponsiveUtils>() (or wrap Get.find in try/catch) to avoid throwing when the dependency is missing; update the code in isScribeInMobileMode to use the cached instance and return false if the dependency is not registered or the lookup fails.scribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dart (1)
24-24: Use the same mobile-mode predicate for icon sizing.Line 24 still keys off
PlatformInfo.isWeb, while Lines 49-55 already switch the flow withAiScribeMobileUtils.isScribeInMobileMode(context). On responsive web, that can leave a desktop-sized trigger paired with the mobile Scribe modal.Suggested change
- final iconSize = PlatformInfo.isWeb ? AIScribeSizes.scribeIcon : AIScribeSizes.scribeMobileIcon; + final isScribeMobile = AiScribeMobileUtils.isScribeInMobileMode(context); + final iconSize = isScribeMobile ? AIScribeSizes.scribeMobileIcon : AIScribeSizes.scribeIcon;Also applies to: 49-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dart` at line 24, The icon sizing uses PlatformInfo.isWeb (iconSize = PlatformInfo.isWeb ? AIScribeSizes.scribeIcon : AIScribeSizes.scribeMobileIcon) which mismatches the mobile predicate used later; change that predicate to AiScribeMobileUtils.isScribeInMobileMode(context) so iconSize uses the same mobile-mode check as the modal flow (retain AIScribeSizes.scribeIcon and AIScribeSizes.scribeMobileIcon), and ensure any other size/flow branches (see the block around the AiScribeMobileUtils.isScribeInMobileMode(context) check at lines ~49-55) consistently use the same predicate.scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart (1)
8-9: Fail fast when a category item has no category handler.At Line 28, a category tap becomes a silent no-op when
onCategorySelectedis null. For a reusable menu item, I'd guard that in the constructor instead of rendering a dead control.Suggested change
const AiScribeMobileActionsItem({ super.key, required this.menuAction, required this.imagePaths, this.onCategorySelected, required this.onActionSelected, - }); + }) : assert( + menuAction is! AiScribeCategoryContextMenuAction || onCategorySelected != null, + 'onCategorySelected is required for category actions', + );Also applies to: 11-17, 27-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart` around lines 8 - 9, The constructor for AiScribeMobileActionsItem should enforce that a category tap handler exists so the widget never renders a no-op; add an assertion or make onCategorySelected required (instead of nullable) in the class constructor and update the field declaration (onCategorySelected / onActionSelected) accordingly so any missing handler fails fast at construction time rather than silently doing nothing in the tap handler (refer to AiScribeMobileActionsItem, onCategorySelected, and onActionSelected to locate and change the declarations and constructor).scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart (1)
123-129: Consider referencingsuggestionContentif intentionally identical.
contentCardTextStyle is identical tosuggestionContent. If they should remain coupled, consider referencing one from the other to avoid drift. If they're expected to diverge, the duplication is fine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart` around lines 123 - 129, contentCard duplicates suggestionContent; to avoid drift either make contentCard refer to suggestionContent (e.g., assign contentCard = suggestionContent) or explicitly document why they differ. Update the declaration of contentCard to reuse suggestionContent (or vice versa) by referencing the existing TextStyle symbol suggestionContent, ensuring any copyWith overrides remain only where intentional.scribe/lib/scribe/ai/data/service/prompt_service.dart (1)
88-95: Consider preserving the original exception for debugging.Catching all exceptions and rethrowing a generic
Exceptionloses the original stack trace and exception type. Consider usingthrow Exception('Prompt not found: $name, cause: $e')or rethrowing the original.♻️ Alternative with better error context
Future<Prompt> getPromptByName(String name) async { final promptData = await loadPrompts(); - try { - return promptData.prompts.firstWhere((prompt) => prompt.name == name); - } catch (_) { - throw Exception('Prompt not found: $name'); - } + final prompt = promptData.prompts.where((p) => p.name == name).firstOrNull; + if (prompt == null) { + throw Exception('Prompt not found: $name'); + } + return prompt; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/data/service/prompt_service.dart` around lines 88 - 95, The catch in getPromptByName currently swallows the original error from promptData.prompts.firstWhere and rethrows a generic Exception; change the catch from a bare (_) to capture the exception (e) and stack (stack) and either rethrow the original exception or throw a new Exception that includes the original error details (e) and stack trace to preserve debugging context; update getPromptByName (and any related error handling around loadPrompts/prompts.firstWhere) to include the original error message and stack when propagating the failure.scribe/test/scribe/ai/domain/model/prompt_data_test.dart (1)
51-62: Consider adding negative test cases for error handling.The test suite covers happy paths well, but consider adding tests for:
PromptData.fromJsonwhenpromptskey is missing (currently handled, but not tested explicitly)Prompt.fromJsonwhennameis missing or not a string (throwsFormatException)💡 Example test cases
test('fromJson should throw FormatException when name is missing', () { final jsonData = { "messages": [] }; expect(() => Prompt.fromJson(jsonData), throwsA(isA<FormatException>())); }); test('fromJson should handle missing prompts key', () { final jsonData = <String, dynamic>{}; final promptData = PromptData.fromJson(jsonData); expect(promptData.prompts, isEmpty); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/test/scribe/ai/domain/model/prompt_data_test.dart` around lines 51 - 62, Add negative unit tests to cover error handling: add a test that verifies Prompt.fromJson throws a FormatException when the "name" field is missing or not a string (call Prompt.fromJson with a map missing "name" and expect throwsA(isA<FormatException>())), and add a test that verifies PromptData.fromJson handles a missing "prompts" key by returning an empty prompts list (call PromptData.fromJson with an empty map and assert promptData.prompts isEmpty). Reference Prompt.fromJson and PromptData.fromJson when locating where to add these new tests.lib/features/mailbox_dashboard/domain/state/get_linagora_ecosystem_state.dart (1)
15-20: Prefer a typed failure payload overdynamic.Using
dynamichere reduces type safety;Object(or a project-specific failure type) is safer and still flexible.♻️ Suggested refactor
class GetLinagoraEcosystemFailure extends Failure { - final dynamic exception; + final Object exception; GetLinagoraEcosystemFailure(this.exception); `@override` List<Object?> get props => [exception]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/mailbox_dashboard/domain/state/get_linagora_ecosystem_state.dart` around lines 15 - 20, Change the untyped payload on the GetLinagoraEcosystemFailure to a typed one instead of dynamic: update the field and constructor parameter "exception" to use Object (or a project-specific Failure/Error type) and keep List<Object?> get props => [exception]; then adjust any callers of GetLinagoraEcosystemFailure to pass values matching the new type; ensure imports/types are updated if you use a specific failure class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/lib/utils/string_convert.dart`:
- Around line 260-263: convertTextContentToHtmlContent currently inserts raw
text into HTML; update this function to HTML-escape special characters (&, <, >,
", ') before replacing newlines so callers cannot inject markup or scripts.
Implement escaping inside convertTextContentToHtmlContent (or call an existing
escape utility) then run replaceAll('\n','<br>') on the escaped string and wrap
with '<div>...</div>'. Keep the function signature the same so callers remain
safe.
In
`@lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart`:
- Around line 117-157: Both restoreSelection and getSavedSelection return
dynamic results from JS evaluation and need null-safe string conversion; update
all return points in restoreSelection and getSavedSelection so the results from
richTextWebController?.editorController.evaluateJavascriptWeb(...) and
richTextMobileTabletController?.htmlEditorApi?.webViewController.evaluateJavascript(...)
are converted with ?.toString() ?? "" (apply to the
HtmlUtils.restoreSelection/HtmlUtils.getSavedSelection call returns) so the
methods always return a non-null String.
- Around line 96-115: The saveSelection method returns the raw dynamic result
from evaluateJavascript/evaluateJavascriptWeb which may be null or non-string;
update saveSelection to safely normalize the dynamic result to a String by
checking for null and non-string types (e.g., capture the result from
richTextWebController?.editorController.evaluateJavascriptWeb and
richTextMobileTabletController?.htmlEditorApi?.webViewController.evaluateJavascript
into a local variable, then return result?.toString() ?? "" or perform an
explicit type check/cast) so saveSelection always returns a non-null String;
reference the saveSelection function and the two call sites
richTextWebController?.editorController.evaluateJavascriptWeb and
richTextMobileTabletController?.htmlEditorApi?.webViewController.evaluateJavascript
when making the change.
In
`@lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart`:
- Line 342: The cachedLinagoraEcosystem field is never cleared on failures or
session changes, causing loadLinagoraEcosystem to return early with stale data;
update the failure handler that currently calls _applyScribePromptUrl(null) to
also set cachedLinagoraEcosystem = null (or a cleared sentinel) so the fetch can
retry, and ensure handleReloaded and onClose clear cachedLinagoraEcosystem as
well so session/account transitions do not retain stale ecosystem/prompt-url
values.
In `@scribe/lib/scribe/ai/data/service/prompt_service.dart`:
- Around line 61-62: In PromptService::_fetchPromptsFromUrl, avoid logging the
raw URL; parse the incoming URL (Uri.parse(url)) and log a sanitized version
instead—either the scheme+host+path or the full URL with query parameter values
masked (e.g., replace each query value with **** or remove query altogether)
before calling log, so sensitive tokens in query strings are never emitted.
In `@scribe/lib/scribe/ai/domain/constants/ai_prompts.dart`:
- Line 8: The static field _promptService in AIPrompts is eagerly initialized
via Get.find<PromptService>() which can throw if DI
(NetworkBindings.dependencies()) hasn't run; change it to lazy/resilient lookup
by replacing the static final initialization with a getter or lazy accessor that
calls Get.find<PromptService>() inside a try/catch and falls back to throwing a
clearer error or returning null/throwing a controlled exception; update all
references in AIPrompts to use the new getter/accessor (or nullable service) so
access is safe even if DI isn't set up yet.
In `@scribe/lib/scribe/ai/domain/repository/ai_scribe_repository.dart`:
- Line 1: The domain repository imports the data-layer DTO AiMessage, coupling
domain to data models; remove the import of ai/data/model/ai_message.dart from
the AiScribeRepository interface and replace usages of that DTO with a
domain-facing type (e.g., a domain AiMessage entity or a generic
Message/MessageDto interface declared in the domain package) in the
AiScribeRepository contract. Keep mapping logic in the data layer implementation
(map between the data ai_message model and the domain model inside the concrete
repository), and ensure method signatures in AiScribeRepository reference only
domain types so the domain layer has no dependency on
ai/data/model/ai_message.dart.
In
`@scribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_suggestion_state_mixin.dart`:
- Around line 42-67: Persist the effective request parameters so retries and UI
decisions use the same action/content that produced the current suggestion: in
loadSuggestion, after computing aiActionToSend and contentToSend, assign them
into the mixin's state fields (e.g., set a storedAiAction and storedContent
properties or update existing getters backing fields) before calling
_interactor!.execute; ensure subsequent logic that computes hasContent and the
retry callback (the retry that calls loadSuggestion() with no args) reads from
those storedAiAction/storedContent values instead of the original
aiAction/content getters so retry reproduces the identical request and the UI
derives its state from the persisted parameters.
In
`@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart`:
- Around line 188-223: The sheet currently hides the entire action list when
hasContent is false which results in a blank body; update the UI to handle the
"no selection / no content" state explicitly by always including the
ValueListenableBuilder for _selectedCategory (instead of gating it behind if
(hasContent)) and inside its builder check hasContent to either return
_buildMenuListView(menuActions) / _buildSubmenuListView() when content exists or
return a clear empty-state widget (e.g., descriptive text + disabled action
placeholders or a lightweight illustration) when content is empty; ensure
showCustomPromptBar behavior remains consistent by still showing the prompt bar
when enabled and use the same symbols (_buildMenuListView,
_buildSubmenuListView, _selectedCategory, hasContent, showCustomPromptBar) to
locate and implement the change.
In `@scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_modal_widget.dart`:
- Around line 62-98: The anchored height calculation still always includes
searchBarHeight which misreports the modal size when showCustomPromptBar is
false; update the computation used before calling
AnchoredModalLayoutCalculator.calculate so that searchBarHeight is 0 (or
otherwise omitted) when showCustomPromptBar == false—adjust the searchBarHeight
variable assignment (used to compute maxHeightModal) to depend on
showCustomPromptBar, and ensure maxHeightModal (and related hasContent logic)
uses that adjusted value before creating AnchoredModalLayoutInput and calling
AnchoredModalLayoutCalculator.calculate.
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart`:
- Around line 61-80: The computed modalMaxHeight can drop below
AIScribeSizes.suggestionModalMinHeight when the keyboard reduces
availableHeight; update the calculation where modalMaxHeight is derived (which
uses availableHeight and AIScribeSizes.suggestionModalMaxHeight) to ensure it is
clamped to at least AIScribeSizes.suggestionModalMinHeight so the BoxConstraints
used by _buildModalContainer() remain valid; specifically, after computing the
min(...) result for modalMaxHeight, apply a max/clamp with
AIScribeSizes.suggestionModalMinHeight so modalMaxHeight =
max(AIScribeSizes.suggestionModalMinHeight, previousValue).
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dart`:
- Around line 47-52: The replace button in _buildReplaceButton currently uses
desktop-only constraints (Container with BoxConstraints and fixed height) while
the insert button adapts for mobile; update _buildReplaceButton to apply the
same mobile sizing logic as the insert action (use the same MediaQuery/isMobile
check and wrap/layout as _buildInsertButton) so when hasContent is true both
buttons use AIScribeSizes.minButtonWidth and AIScribeSizes.buttonHeight
consistently on mobile; ensure you reference and reuse the same sizing
logic/variables (AIScribeSizes.minButtonWidth, AIScribeSizes.buttonHeight) and
the same widget structure used by the insert button to keep dimensions
identical.
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_toolbar.dart`:
- Around line 23-39: The build currently calls Get.find<AppToast>() unguarded
which throws if the binding isn't registered; change it to check
Get.isRegistered<AppToast>() first and only call Get.find when available (e.g.,
final appToast = Get.isRegistered<AppToast>() ? Get.find<AppToast>() : null),
then guard the call to appToast.showToastSuccessMessage(...) so the copy button
still sets the Clipboard (Clipboard.setData(...)) even when appToast is null;
update references to appToast in the onTapActionCallback accordingly to avoid
null dereference.
In `@scribe/test/scribe/ai/data/service/prompt_service_test.dart`:
- Around line 61-109: Tests in the "PromptService getPromptByName" group are
directly querying PromptData.prompts instead of exercising PromptService; update
both tests to call PromptService.getPromptByName (or the appropriate service
constructor/factory that accepts a PromptData instance) so you validate service
behavior, e.g., instantiate PromptService with the test PromptData, call
PromptService.getPromptByName('test-prompt') and assert name/messages, and for
the negative test call PromptService.getPromptByName('non-existent-prompt') and
assert it throws; reference PromptService.getPromptByName, PromptService
(constructor), and PromptData/Prompt to locate where to change the assertions
and calls.
---
Outside diff comments:
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart`:
- Around line 118-188: In _buildAnchoredLayout: the mobile branch
(_isMobileView) currently pins top to anchorPos.dy and computes width from
anchorPos.dx which causes negative widths and overlap for right-aligned entry
points; instead use the resolved AnchoredModalLayoutCalculator result (layout)
for positioning and sizing on mobile as well — set top = layout.top (or
top/bottom from layout as available), bottom = layout.bottom if provided, height
= layout.availableHeight, and compute width from the layout's left/right values
or modalWidth clamped to available space (e.g., use layout.left and screen width
or any layout-provided availableWidth) so the popup honors the calculated
placement; update references in _buildAnchoredLayout to use layout.* fields
rather than anchorPos.* for the mobile branch.
---
Nitpick comments:
In `@core/lib/utils/html/html_utils.dart`:
- Around line 155-161: The hard-coded offsets { x: 24, y: -24 } used when
computing buttonOffset in the block that sets x/y (referencing isWebPlatform,
buttonOffset, lastRect, editableRect) should be extracted to named constants
(e.g., MOBILE_BUTTON_OFFSET_X and MOBILE_BUTTON_OFFSET_Y or a single
MOBILE_BUTTON_OFFSET constant) declared near the top of the file or passed as
parameters; replace the literals with those constants, add a short comment
describing why they exist ("arbitrary values to avoid selection marks on
Android/iOS"), and ensure any tests or callers that rely on this behavior are
updated to use the new constants or parameters.
In
`@lib/features/composer/presentation/controller/rich_text_mobile_tablet_controller.dart`:
- Around line 24-27: The injected JS that calls
document.getElementById('editor').focus() should first guard that the element
exists to avoid transient JS exceptions; update the evaluateJavascript call (the
call on htmlEditorApi?.webViewController.evaluateJavascript) to run a small IIFE
that queries document.getElementById('editor') into a variable and only calls
.focus() when that variable is non-null (e.g., if (el) el.focus()), so the
Dart-side catch no longer sees noisy timing warnings.
In
`@lib/features/mailbox_dashboard/domain/state/get_linagora_ecosystem_state.dart`:
- Around line 15-20: Change the untyped payload on the
GetLinagoraEcosystemFailure to a typed one instead of dynamic: update the field
and constructor parameter "exception" to use Object (or a project-specific
Failure/Error type) and keep List<Object?> get props => [exception]; then adjust
any callers of GetLinagoraEcosystemFailure to pass values matching the new type;
ensure imports/types are updated if you use a specific failure class.
In `@scribe/lib/scribe/ai/data/service/prompt_service.dart`:
- Around line 88-95: The catch in getPromptByName currently swallows the
original error from promptData.prompts.firstWhere and rethrows a generic
Exception; change the catch from a bare (_) to capture the exception (e) and
stack (stack) and either rethrow the original exception or throw a new Exception
that includes the original error details (e) and stack trace to preserve
debugging context; update getPromptByName (and any related error handling around
loadPrompts/prompts.firstWhere) to include the original error message and stack
when propagating the failure.
In `@scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart`:
- Around line 123-129: contentCard duplicates suggestionContent; to avoid drift
either make contentCard refer to suggestionContent (e.g., assign contentCard =
suggestionContent) or explicitly document why they differ. Update the
declaration of contentCard to reuse suggestionContent (or vice versa) by
referencing the existing TextStyle symbol suggestionContent, ensuring any
copyWith overrides remain only where intentional.
In `@scribe/lib/scribe/ai/presentation/utils/ai_scribe_mobile_utils.dart`:
- Around line 6-8: The method isScribeInMobileMode calls
Get.find<ResponsiveUtils>() twice; cache the lookup by retrieving the
ResponsiveUtils instance once (e.g., assign to a local variable inside
isScribeInMobileMode) and use that for both isMobile and isLandscapeMobile
checks, and guard the lookup with Get.isRegistered<ResponsiveUtils>() (or wrap
Get.find in try/catch) to avoid throwing when the dependency is missing; update
the code in isScribeInMobileMode to use the cached instance and return false if
the dependency is not registered or the lookup fails.
In
`@scribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dart`:
- Line 24: The icon sizing uses PlatformInfo.isWeb (iconSize =
PlatformInfo.isWeb ? AIScribeSizes.scribeIcon : AIScribeSizes.scribeMobileIcon)
which mismatches the mobile predicate used later; change that predicate to
AiScribeMobileUtils.isScribeInMobileMode(context) so iconSize uses the same
mobile-mode check as the modal flow (retain AIScribeSizes.scribeIcon and
AIScribeSizes.scribeMobileIcon), and ensure any other size/flow branches (see
the block around the AiScribeMobileUtils.isScribeInMobileMode(context) check at
lines ~49-55) consistently use the same predicate.
In
`@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart`:
- Around line 8-9: The constructor for AiScribeMobileActionsItem should enforce
that a category tap handler exists so the widget never renders a no-op; add an
assertion or make onCategorySelected required (instead of nullable) in the class
constructor and update the field declaration (onCategorySelected /
onActionSelected) accordingly so any missing handler fails fast at construction
time rather than silently doing nothing in the tap handler (refer to
AiScribeMobileActionsItem, onCategorySelected, and onActionSelected to locate
and change the declarations and constructor).
In `@scribe/test/scribe/ai/domain/model/prompt_data_test.dart`:
- Around line 51-62: Add negative unit tests to cover error handling: add a test
that verifies Prompt.fromJson throws a FormatException when the "name" field is
missing or not a string (call Prompt.fromJson with a map missing "name" and
expect throwsA(isA<FormatException>())), and add a test that verifies
PromptData.fromJson handles a missing "prompts" key by returning an empty
prompts list (call PromptData.fromJson with an empty map and assert
promptData.prompts isEmpty). Reference Prompt.fromJson and PromptData.fromJson
when locating where to add these new tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d40850cc-3440-4ea2-ac3f-dcfefc593b1b
⛔ Files ignored due to path filters (2)
assets/images/ic_arrow_back_ios.svgis excluded by!**/*.svgpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
core/lib/presentation/resources/image_paths.dartcore/lib/utils/html/html_utils.dartcore/lib/utils/string_convert.dartlib/features/base/mixin/ai_scribe_mixin.dartlib/features/composer/presentation/composer_view.dartlib/features/composer/presentation/composer_view_web.dartlib/features/composer/presentation/controller/rich_text_mobile_tablet_controller.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/composer/presentation/widgets/ai_scribe/composer_ai_scribe_selection_overlay.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/composer/presentation/widgets/mobile/tablet_bottom_bar_composer_widget.dartlib/features/composer/presentation/widgets/web/mobile_responsive_app_bar_composer_widget.dartlib/features/composer/presentation/widgets/web/web_editor_widget.dartlib/features/mailbox_dashboard/data/repository/linagora_ecosystem_repository_impl.dartlib/features/mailbox_dashboard/domain/linagora_ecosystem/converters/linagora_ecosystem_converter.dartlib/features/mailbox_dashboard/domain/linagora_ecosystem/linagora_ecosystem.dartlib/features/mailbox_dashboard/domain/linagora_ecosystem/linagora_ecosystem_identifier.dartlib/features/mailbox_dashboard/domain/repository/linagora_ecosystem_repository.dartlib/features/mailbox_dashboard/domain/state/get_linagora_ecosystem_state.dartlib/features/mailbox_dashboard/domain/usecases/get_linagora_system_interactor.dartlib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_cached_ai_scribe_extension.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_scribe_prompt_url_extension.dartlib/features/manage_account/presentation/preferences/preferences_controller.dartlib/main/bindings/network/network_bindings.dartscribe/assets/prompts.jsonscribe/lib/scribe.dartscribe/lib/scribe/ai/data/datasource/ai_datasource.dartscribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dartscribe/lib/scribe/ai/data/model/ai_message.dartscribe/lib/scribe/ai/data/network/ai_api.dartscribe/lib/scribe/ai/data/repository/ai_repository_impl.dartscribe/lib/scribe/ai/data/service/prompt_service.dartscribe/lib/scribe/ai/domain/constants/ai_prompts.dartscribe/lib/scribe/ai/domain/model/prompt_data.dartscribe/lib/scribe/ai/domain/repository/ai_scribe_repository.dartscribe/lib/scribe/ai/domain/usecases/generate_ai_text_interactor.dartscribe/lib/scribe/ai/l10n/intl_ar.arbscribe/lib/scribe/ai/l10n/intl_de.arbscribe/lib/scribe/ai/l10n/intl_en.arbscribe/lib/scribe/ai/l10n/intl_fr.arbscribe/lib/scribe/ai/l10n/intl_it.arbscribe/lib/scribe/ai/l10n/intl_messages.arbscribe/lib/scribe/ai/l10n/intl_ru.arbscribe/lib/scribe/ai/l10n/intl_vi.arbscribe/lib/scribe/ai/localizations/scribe_localizations.dartscribe/lib/scribe/ai/presentation/model/ai_action.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/ai_scribe_mobile_utils.dartscribe/lib/scribe/ai/presentation/utils/context_menu/context_submenu_controller.dartscribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_modal_manager.dartscribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_suggestion_state_mixin.dartscribe/lib/scribe/ai/presentation/utils/modal/anchored_modal_layout_calculator.dartscribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dartscribe/lib/scribe/ai/presentation/widgets/context_menu/ai_scribe_context_menu.dartscribe/lib/scribe/ai/presentation/widgets/context_menu/ai_scribe_context_menu_item.dartscribe/lib/scribe/ai/presentation/widgets/context_menu/ai_scribe_submenu.dartscribe/lib/scribe/ai/presentation/widgets/items/ai_scribe_menu_icon.dartscribe/lib/scribe/ai/presentation/widgets/items/ai_scribe_menu_item.dartscribe/lib/scribe/ai/presentation/widgets/items/ai_scribe_menu_submenu_icon.dartscribe/lib/scribe/ai/presentation/widgets/items/ai_scribe_menu_text.dartscribe/lib/scribe/ai/presentation/widgets/items/ai_scribe_submenu_item.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.dartscribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_modal_widget.dartscribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_improve_button.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_list_actions.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_toolbar.dartscribe/lib/scribe/ai/presentation/widgets/overlay/ai_selection_overlay.dartscribe/lib/scribe/ai/presentation/widgets/search/ai_scribe_bar.dartscribe/pubspec.yamlscribe/test/assets/test_prompts.jsonscribe/test/scribe/ai/data/service/prompt_service_test.dartscribe/test/scribe/ai/domain/model/prompt_data_test.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
...tures/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
Show resolved
Hide resolved
...tures/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
Show resolved
Hide resolved
lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_modal_widget.dart
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart
Show resolved
Hide resolved
...ib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dart
Outdated
Show resolved
Hide resolved
...ib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_toolbar.dart
Outdated
Show resolved
Hide resolved
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4382. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart (1)
202-221:⚠️ Potential issue | 🟠 MajorKeep predefined actions visible when no content is selected.
At Line 202, gating the full body with
if (hasContent)hides category/actions in no-selection flows. This reintroduces the empty/near-empty sheet state.✅ Suggested fix
- if (hasContent) - Expanded( - child: SingleChildScrollView( - child: Column( - mainAxisSize: MainAxisSize.min, - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - _buildTextCard(widget.content ?? ''), - ValueListenableBuilder<AiScribeCategoryContextMenuAction?>( - valueListenable: _selectedCategory, - builder: (context, selectedCategory, _) { - return selectedCategory == null - ? _buildMenuListView(menuActions) - : _buildSubmenuListView(); - }, - ), - ], - ), - ), - ), + Expanded( + child: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + if (hasContent) _buildTextCard(widget.content!), + ValueListenableBuilder<AiScribeCategoryContextMenuAction?>( + valueListenable: _selectedCategory, + builder: (context, selectedCategory, _) { + return selectedCategory == null + ? _buildMenuListView(menuActions) + : _buildSubmenuListView(); + }, + ), + ], + ), + ), + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart` around lines 202 - 221, The current conditional "if (hasContent)" wraps the whole Expanded body and hides category/action UI when nothing is selected; instead, always render the actions area and only conditionally render the text card. Update the widget tree so Expanded contains the SingleChildScrollView with a Column that shows _buildTextCard(widget.content ?? '') only when hasContent is true, but always includes the ValueListenableBuilder using _selectedCategory to choose between _buildMenuListView(menuActions) and _buildSubmenuListView(); keep menuActions, _selectedCategory, _buildMenuListView, _buildSubmenuListView, and _buildTextCard as the referenced symbols to locate the change.
🧹 Nitpick comments (7)
scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart (1)
123-129: Avoid duplicatingTextStyleconfig forcontentCard.
contentCardis currently identical tosuggestionContent, which creates drift risk during future typography updates. Reuse the existing token directly.♻️ Proposed refactor
- static final TextStyle contentCard = - ThemeUtils.textStyleInter400.copyWith( - fontSize: 14, - height: 22 / 14, - letterSpacing: 0.4, - color: Colors.black.withValues(alpha: 0.85), - ); + static final TextStyle contentCard = suggestionContent;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart` around lines 123 - 129, contentCard duplicates the exact TextStyle of suggestionContent; replace the duplicated config by reusing the existing token to avoid drift — update the static declaration for contentCard to reference the existing suggestionContent TextStyle (use the same symbol instead of copyWith-duplicating the values) so both share the same instance and future typography changes to suggestionContent automatically apply to contentCard.scribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dart (1)
23-23: Consider passingisScribeMobileto avoid duplicate computation.
isScribeMobileis computed twice: once inbuild()(line 23) and again in_onTapActionCallback(line 51). While the current approach is functionally correct, you could pass the value as a parameter to reduce redundancy.♻️ Optional: Pass isScribeMobile to the callback
- onTapActionCallback: () => _onTapActionCallback(context), + onTapActionCallback: () => _onTapActionCallback(context, isScribeMobile), ); } - Future<void> _onTapActionCallback(BuildContext context) async { + Future<void> _onTapActionCallback( + BuildContext context, + bool isScribeMobile, + ) async { final renderBox = context.findRenderObject(); Offset? position; Size? size; if (renderBox != null && renderBox is RenderBox) { position = renderBox.localToGlobal(Offset.zero); size = renderBox.size; } - final isScribeMobile = AiScribeMobileUtils.isScribeInMobileMode(context); - await onTapFallback?.call();Also applies to: 51-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dart` at line 23, Compute isScribeMobile once in build() and pass it into _onTapActionCallback to avoid duplicate calls to AiScribeMobileUtils.isScribeInMobileMode; update the callback signature (e.g., _onTapActionCallback(BuildContext context, bool isScribeMobile, ...)) and all call sites inside the widget so _onTapActionCallback uses the passed isScribeMobile value instead of recomputing it.scribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dart (1)
47-96: Extract a shared action-button builder to avoid drift.
_buildReplaceButtonand_buildInsertButtonduplicate the same sizing/container and tap flow. A private helper would keep mobile sizing and pop/select behavior consistent in one place.♻️ Proposed refactor
+ Widget _buildActionButton({ + required BuildContext context, + required String label, + Color? backgroundColor, + required Color textColor, + required AiScribeSuggestionActions action, + }) { + final isMobileScribe = AiScribeMobileUtils.isScribeInMobileMode(context); + return Flexible( + child: Container( + constraints: BoxConstraints( + minWidth: isMobileScribe + ? AIScribeSizes.minButtonMobileWidth + : AIScribeSizes.minButtonWidth, + ), + height: isMobileScribe + ? AIScribeSizes.buttonMobileHeight + : AIScribeSizes.buttonHeight, + child: ConfirmDialogButton( + label: label, + backgroundColor: backgroundColor, + textColor: textColor, + onTapAction: () { + Navigator.of(context).pop(); + onSelectAction(action, suggestionText); + }, + ), + ), + ); + } + Widget _buildReplaceButton(BuildContext context) { final localizations = ScribeLocalizations.of(context); - final isMobileScribe = AiScribeMobileUtils.isScribeInMobileMode(context); - return Flexible( - child: Container( - constraints: BoxConstraints( - minWidth: isMobileScribe - ? AIScribeSizes.minButtonMobileWidth - : AIScribeSizes.minButtonWidth, - ), - height: isMobileScribe - ? AIScribeSizes.buttonMobileHeight - : AIScribeSizes.buttonHeight, - child: ConfirmDialogButton( - label: AiScribeSuggestionActions.replace.getLabel(localizations), - textColor: AppColor.primaryMain, - onTapAction: () { - Navigator.of(context).pop(); - onSelectAction( - AiScribeSuggestionActions.replace, - suggestionText, - ); - }, - ), - ), + return _buildActionButton( + context: context, + label: AiScribeSuggestionActions.replace.getLabel(localizations), + textColor: AppColor.primaryMain, + action: AiScribeSuggestionActions.replace, ); } Widget _buildInsertButton(BuildContext context) { final localizations = ScribeLocalizations.of(context); - final isMobileScribe = AiScribeMobileUtils.isScribeInMobileMode(context); - return Flexible( - child: Container( - constraints: BoxConstraints(minWidth: isMobileScribe ? AIScribeSizes.minButtonMobileWidth : AIScribeSizes.minButtonWidth), - height: isMobileScribe ? AIScribeSizes.buttonMobileHeight : AIScribeSizes.buttonHeight, - child: ConfirmDialogButton( - label: AiScribeSuggestionActions.insert.getLabel(localizations), - backgroundColor: AppColor.primaryMain, - textColor: Colors.white, - onTapAction: () { - Navigator.of(context).pop(); - onSelectAction( - AiScribeSuggestionActions.insert, - suggestionText, - ); - }, - ), - ), + return _buildActionButton( + context: context, + label: AiScribeSuggestionActions.insert.getLabel(localizations), + backgroundColor: AppColor.primaryMain, + textColor: Colors.white, + action: AiScribeSuggestionActions.insert, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dart` around lines 47 - 96, Replace the duplicated UI and tap logic in _buildReplaceButton and _buildInsertButton with a private helper (e.g., _buildActionButton) that takes parameters: label (String), action (AiScribeSuggestionActions), optional backgroundColor and textColor, and uses AiScribeMobileUtils.isScribeInMobileMode to compute minWidth/height using AIScribeSizes and creates the Container->ConfirmDialogButton; inside the helper call Navigator.of(context).pop() and then onSelectAction(action, suggestionText) so both _buildReplaceButton and _buildInsertButton simply call _buildActionButton with their respective labels, colors, and actions to avoid drift and keep sizing/tap behavior centralized.scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart (1)
26-31: Rename the callback parameter to avoid shadowing.At Line 26, the callback parameter
menuActionshadows the widget fieldmenuAction, which makes this branch harder to read.♻️ Suggested cleanup
- onSelectAction: (menuAction) { - if (menuAction is AiScribeCategoryContextMenuAction) { - onCategorySelected.call(menuAction); + onSelectAction: (selectedAction) { + if (selectedAction is AiScribeCategoryContextMenuAction) { + onCategorySelected.call(selectedAction); } else { - onActionSelected.call(menuAction); + onActionSelected.call(selectedAction); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart` around lines 26 - 31, The closure passed to onSelectAction is using a parameter named menuAction which shadows the widget field menuAction; rename the closure parameter (e.g., selectedAction or action) and update the conditional to check if selectedAction is AiScribeCategoryContextMenuAction and call onCategorySelected(selectedAction) or onActionSelected(selectedAction) accordingly so the field and parameter names are distinct; ensure references to AiScribeCategoryContextMenuAction, onCategorySelected, and onActionSelected are updated to use the new parameter name.scribe/lib/scribe/ai/domain/constants/ai_prompts.dart (1)
8-12: Lazy initialization addresses past concern, but consider adding error handling.The lazy getter pattern resolves the eager initialization issue from the previous review. However, based on learnings,
Get.find<T>()throws if the dependency is unavailable. Consider wrapping in try/catch or checkingGet.isRegistered<PromptService>()for graceful failure.🛡️ Optional: Add defensive check
static PromptService get _promptService { + if (!Get.isRegistered<PromptService>()) { + throw StateError('PromptService not registered. Ensure NetworkBindings.dependencies() has been called.'); + } return _promptServiceInstance ??= Get.find<PromptService>(); }Based on learnings: "wrap Get.find() calls in try/catch because they throw if a dependency is unavailable."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/domain/constants/ai_prompts.dart` around lines 8 - 12, The lazy getter _promptService currently calls Get.find<PromptService>() which throws if the dependency isn't registered; update _promptService to defensively handle that by checking Get.isRegistered<PromptService>() before calling Get.find or wrapping Get.find in try/catch, and if missing return a clear fallback (e.g., throw a descriptive exception or return null/alternative) while keeping the backing field _promptServiceInstance and type PromptService consistent; ensure any thrown message names PromptService and Get.find so it's easy to debug.scribe/lib/scribe/ai/domain/model/prompt_data.dart (1)
50-58: Consider handling unknown roles explicitly.The
buildPromptmethod silently drops messages with roles other thansystemoruser. If new roles are added toAIRoleenum in the future, this could cause unexpected message loss. Consider adding an else clause to handle or log unknown roles.🛡️ Optional: Make role handling exhaustive
List<AIMessage> buildPrompt(String inputText, {String? task}) { return [ for (final message in messages) if (message.role == AIRole.system) AIMessage.ofSystem(message.content) else if (message.role == AIRole.user) AIMessage.ofUser(_replacePlaceholders(message.content, inputText, task)) + // Uncomment if new roles should be preserved as-is: + // else + // message ]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/domain/model/prompt_data.dart` around lines 50 - 58, The buildPrompt method currently drops messages whose message.role is not AIRole.system or AIRole.user; update buildPrompt to handle unknown AIRole values explicitly by adding a final else branch (in the for loop over messages) that throws an UnsupportedError (or logs a warning) including the unexpected role and message content so unknown roles are surfaced instead of silently dropped; reference the buildPrompt method and the AIRole enum to locate the change and ensure the error message includes message.role and message.content.lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart (1)
242-258: ComputeisScribeMobileonce per modal-open flow.Line 242 and Line 257 evaluate
isScribeMobileseparately across awaited operations. Capturing once avoids context-dependent drift and keeps modal behavior deterministic.♻️ Proposed refactor
Future<void> openAIAssistantModal(Offset? position, Size? size) async { clearFocusRecipients(); clearFocusSubject(); - if (isScribeMobile) { + final scribeMobile = isScribeMobile; + if (scribeMobile) { await saveAndUnfocusForModal(); } @@ await AiScribeModalManager.showAIScribeModal( @@ - isScribeMobile: isScribeMobile, + isScribeMobile: scribeMobile, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart` around lines 242 - 258, Compute and capture the isScribeMobile value once before any awaits and reuse it for all subsequent logic: evaluate isScribeMobile into a local final (e.g., final isScribeMobile = ...) before calling saveAndUnfocusForModal() and _getTextOnlyContentInEditor(), then pass that same variable into AiScribeModalManager.showAIScribeModal; update references to isScribeMobile in this flow so the modal behavior is deterministic across the awaited operations (functions to locate: saveAndUnfocusForModal, _getTextOnlyContentInEditor, AiScribeModalManager.showAIScribeModal).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart`:
- Around line 178-186: In unfocusEditor(), wrap the platform channel awaits
(calls on richTextMobileTabletController?.htmlEditorApi — specifically
editorApi.unfocus() and editorApi.hideKeyboard()) in try/catch blocks so any
thrown errors are caught and do not propagate to callers like
saveAndUnfocusForModal() or openAIAssistantModal(); catch and log the error (or
silently ignore) and continue so unfocus failures on iOS/Android don’t block
modal opening. Ensure the existing PlatformInfo.isIOS / isAndroid branches keep
their awaits but the calls are guarded by the try/catch around each
platform-specific editorApi call.
- Around line 202-206: The code mixes PlatformInfo.isMobile and isScribeMobile
leading to inconsistent save/restore/clear selection behavior; unify to a single
mobile-mode predicate by computing a local bool (e.g., useMobile =
isScribeMobile) at the start of the handler and replace all occurrences of
PlatformInfo.isMobile with that predicate for the selection lifecycle calls
(ensureMobileEditorFocused, restoreSelection, clearSelection and any save/get
selection branches) so save/restore/clear use the same mobile-mode decision
throughout.
In `@scribe/lib/scribe/ai/domain/model/prompt_data.dart`:
- Around line 60-68: The _replacePlaceholders function currently leaves the
literal "{{task}}" in output when task is null; update it so the task
placeholder is always removed by replacing '{{task}}' with an empty string when
task is null (e.g., replace '{{task}}' with task ?? ''), keeping the existing
replacement for '{{input}}' and returning the cleaned result; reference the
_replacePlaceholders function and variables content, inputText, and task when
making the change.
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart`:
- Around line 154-162: The computed height and width near the screen edge can be
smaller than the component minimums causing invalid BoxConstraints; update the
calculations where height and width are set (using layout.availableHeight,
screenSize, anchorPos, anchorSize, _defaultPadding, and modalWidth) to clamp
each value to at least the minimums used by the modal (e.g.
AIScribeSizes.suggestionModalMinHeight and the corresponding min width constant)
and at most the available space — i.e. wrap the current min(...) results with a
max(..., AIScribeSizes.suggestionModalMinHeight) for height (and equivalent for
width) so _buildModalContainer never receives max < min.
In `@scribe/test/scribe/ai/data/service/prompt_service_test.dart`:
- Around line 70-78: The test uses expect(() =>
service.getPromptByName('non-existent-prompt'), throwsException) which doesn't
await the Future and won't catch async errors; change the assertion to await the
Future-returning call or pass the Future directly to expect so the async
exception is observed — e.g., use
expect(service.getPromptByName('non-existent-prompt'), throwsException) or wrap
in an async closure like expect(() async => await
service.getPromptByName('non-existent-prompt'), throwsException); update the
test that constructs PromptService with _throwingDio() and the getPromptByName
invocation accordingly.
---
Duplicate comments:
In
`@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_bottom_sheet.dart`:
- Around line 202-221: The current conditional "if (hasContent)" wraps the whole
Expanded body and hides category/action UI when nothing is selected; instead,
always render the actions area and only conditionally render the text card.
Update the widget tree so Expanded contains the SingleChildScrollView with a
Column that shows _buildTextCard(widget.content ?? '') only when hasContent is
true, but always includes the ValueListenableBuilder using _selectedCategory to
choose between _buildMenuListView(menuActions) and _buildSubmenuListView(); keep
menuActions, _selectedCategory, _buildMenuListView, _buildSubmenuListView, and
_buildTextCard as the referenced symbols to locate the change.
---
Nitpick comments:
In
`@lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart`:
- Around line 242-258: Compute and capture the isScribeMobile value once before
any awaits and reuse it for all subsequent logic: evaluate isScribeMobile into a
local final (e.g., final isScribeMobile = ...) before calling
saveAndUnfocusForModal() and _getTextOnlyContentInEditor(), then pass that same
variable into AiScribeModalManager.showAIScribeModal; update references to
isScribeMobile in this flow so the modal behavior is deterministic across the
awaited operations (functions to locate: saveAndUnfocusForModal,
_getTextOnlyContentInEditor, AiScribeModalManager.showAIScribeModal).
In `@scribe/lib/scribe/ai/domain/constants/ai_prompts.dart`:
- Around line 8-12: The lazy getter _promptService currently calls
Get.find<PromptService>() which throws if the dependency isn't registered;
update _promptService to defensively handle that by checking
Get.isRegistered<PromptService>() before calling Get.find or wrapping Get.find
in try/catch, and if missing return a clear fallback (e.g., throw a descriptive
exception or return null/alternative) while keeping the backing field
_promptServiceInstance and type PromptService consistent; ensure any thrown
message names PromptService and Get.find so it's easy to debug.
In `@scribe/lib/scribe/ai/domain/model/prompt_data.dart`:
- Around line 50-58: The buildPrompt method currently drops messages whose
message.role is not AIRole.system or AIRole.user; update buildPrompt to handle
unknown AIRole values explicitly by adding a final else branch (in the for loop
over messages) that throws an UnsupportedError (or logs a warning) including the
unexpected role and message content so unknown roles are surfaced instead of
silently dropped; reference the buildPrompt method and the AIRole enum to locate
the change and ensure the error message includes message.role and
message.content.
In `@scribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dart`:
- Around line 123-129: contentCard duplicates the exact TextStyle of
suggestionContent; replace the duplicated config by reusing the existing token
to avoid drift — update the static declaration for contentCard to reference the
existing suggestionContent TextStyle (use the same symbol instead of
copyWith-duplicating the values) so both share the same instance and future
typography changes to suggestionContent automatically apply to contentCard.
In
`@scribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dart`:
- Line 23: Compute isScribeMobile once in build() and pass it into
_onTapActionCallback to avoid duplicate calls to
AiScribeMobileUtils.isScribeInMobileMode; update the callback signature (e.g.,
_onTapActionCallback(BuildContext context, bool isScribeMobile, ...)) and all
call sites inside the widget so _onTapActionCallback uses the passed
isScribeMobile value instead of recomputing it.
In
`@scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart`:
- Around line 26-31: The closure passed to onSelectAction is using a parameter
named menuAction which shadows the widget field menuAction; rename the closure
parameter (e.g., selectedAction or action) and update the conditional to check
if selectedAction is AiScribeCategoryContextMenuAction and call
onCategorySelected(selectedAction) or onActionSelected(selectedAction)
accordingly so the field and parameter names are distinct; ensure references to
AiScribeCategoryContextMenuAction, onCategorySelected, and onActionSelected are
updated to use the new parameter name.
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dart`:
- Around line 47-96: Replace the duplicated UI and tap logic in
_buildReplaceButton and _buildInsertButton with a private helper (e.g.,
_buildActionButton) that takes parameters: label (String), action
(AiScribeSuggestionActions), optional backgroundColor and textColor, and uses
AiScribeMobileUtils.isScribeInMobileMode to compute minWidth/height using
AIScribeSizes and creates the Container->ConfirmDialogButton; inside the helper
call Navigator.of(context).pop() and then onSelectAction(action, suggestionText)
so both _buildReplaceButton and _buildInsertButton simply call
_buildActionButton with their respective labels, colors, and actions to avoid
drift and keep sizing/tap behavior centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7fd85010-aec1-479a-8157-2f09f47a57b4
📒 Files selected for processing (29)
core/lib/utils/string_convert.dartlib/features/composer/presentation/controller/rich_text_mobile_tablet_controller.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/mailbox_dashboard/domain/state/get_linagora_ecosystem_state.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_scribe_prompt_url_extension.dartmodel/lib/extensions/session_extension.dartscribe/lib/scribe/ai/data/datasource/ai_datasource.dartscribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dartscribe/lib/scribe/ai/data/model/ai_api_request.dartscribe/lib/scribe/ai/data/network/ai_api.dartscribe/lib/scribe/ai/data/repository/ai_repository_impl.dartscribe/lib/scribe/ai/data/service/prompt_service.dartscribe/lib/scribe/ai/domain/constants/ai_prompts.dartscribe/lib/scribe/ai/domain/model/ai_message.dartscribe/lib/scribe/ai/domain/model/prompt_data.dartscribe/lib/scribe/ai/domain/repository/ai_scribe_repository.dartscribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dartscribe/lib/scribe/ai/presentation/utils/ai_scribe_mobile_utils.dartscribe/lib/scribe/ai/presentation/utils/modal/ai_scribe_suggestion_state_mixin.dartscribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.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/modal/ai_scribe_modal_widget.dartscribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_toolbar.dartscribe/test/scribe/ai/data/service/prompt_service_test.dartscribe/test/scribe/ai/domain/model/prompt_data_test.dart
🚧 Files skipped from review as they are similar to previous changes (10)
- core/lib/utils/string_convert.dart
- scribe/lib/scribe/ai/data/datasource/ai_datasource.dart
- scribe/test/scribe/ai/domain/model/prompt_data_test.dart
- lib/features/composer/presentation/controller/rich_text_mobile_tablet_controller.dart
- scribe/lib/scribe/ai/data/service/prompt_service.dart
- scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart
- scribe/lib/scribe/ai/presentation/utils/ai_scribe_mobile_utils.dart
- lib/features/mailbox_dashboard/domain/state/get_linagora_ecosystem_state.dart
- scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_modal_widget.dart
- lib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_scribe_prompt_url_extension.dart
...tures/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
Show resolved
Hide resolved
...tures/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scribe/lib/scribe/ai/domain/constants/ai_prompts.dart (1)
34-35: Consider rejecting empty custom tasks early.At Line 35, a whitespace-only
customPromptis forwarded astask, which can produce ambiguous/low-quality requests. A small guard makes this contract explicit.Proposed refactor
static Future<List<AIMessage>> buildCustomPrompt(String customPrompt, String? text) async { - return await _promptService.buildPromptByName(CustomPromptAction.promptId, text ?? '', task: customPrompt); + if (customPrompt.trim().isEmpty) { + throw ArgumentError('Custom prompt cannot be empty'); + } + return await _promptService.buildPromptByName( + CustomPromptAction.promptId, + text ?? '', + task: customPrompt, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scribe/lib/scribe/ai/domain/constants/ai_prompts.dart` around lines 34 - 35, In buildCustomPrompt, validate the customPrompt argument before calling _promptService.buildPromptByName: trim customPrompt and if it's empty or only whitespace, reject early (e.g., throw an ArgumentError or return a Future.error) with a clear message indicating the task was empty; otherwise pass the trimmed value as task to CustomPromptAction.promptId when calling buildPromptByName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart`:
- Around line 63-72: In setTextInEditor, remove the unnecessary await on
richTextMobileTabletController?.htmlEditorApi?.setText(htmlContent) because
html_editor_enhanced's setText returns void (not a Future); keep the
PlatformInfo.isWeb branch using
richTextWebController?.editorController.setText(htmlContent) as-is and call
htmlEditorApi.setText(htmlContent) without awaiting so behavior matches web
branch and avoids awaiting a non-Future.
In
`@scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart`:
- Around line 166-174: The desktop/web branch sets height =
layout.availableHeight without enforcing the minimum, which can make
BoxConstraints invalid; modify the desktop/web branch in
ai_scribe_suggestion_widget.dart (the else block where _isMobileView is false)
to clamp height to at least AIScribeSizes.suggestionModalMinHeight (same
behavior as the mobile branch), using the same min/max logic applied for the
mobile path so _buildModalContainer never receives minHeight > maxHeight.
---
Nitpick comments:
In `@scribe/lib/scribe/ai/domain/constants/ai_prompts.dart`:
- Around line 34-35: In buildCustomPrompt, validate the customPrompt argument
before calling _promptService.buildPromptByName: trim customPrompt and if it's
empty or only whitespace, reject early (e.g., throw an ArgumentError or return a
Future.error) with a clear message indicating the task was empty; otherwise pass
the trimmed value as task to CustomPromptAction.promptId when calling
buildPromptByName.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a0b10e3-96fa-462c-a13a-038a70d56861
📒 Files selected for processing (9)
lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartscribe/lib/scribe/ai/domain/constants/ai_prompts.dartscribe/lib/scribe/ai/domain/model/prompt_data.dartscribe/lib/scribe/ai/presentation/styles/ai_scribe_styles.dartscribe/lib/scribe/ai/presentation/widgets/button/inline_ai_assist_button.dartscribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dartscribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dartscribe/lib/scribe/ai/presentation/widgets/modal/suggestion/ai_scribe_suggestion_success_actions.dartscribe/test/scribe/ai/data/service/prompt_service_test.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- scribe/test/scribe/ai/data/service/prompt_service_test.dart
- scribe/lib/scribe/ai/domain/model/prompt_data.dart
- scribe/lib/scribe/ai/presentation/widgets/mobile/ai_scribe_mobile_actions_item.dart
...tures/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
Show resolved
Hide resolved
scribe/lib/scribe/ai/presentation/widgets/modal/ai_scribe_suggestion_widget.dart
Show resolved
Hide resolved
Member
|
hi @zatteo now we have CORS issue with getting prompt (linagora env). Can you take a look? 
|
Member
Hi, normally it should just happen locally because with use localhost:XXXX. But writing this, it makes me think that it may not work will all production domain name. Let me check with infra. In the meantime, you can try with |
hoangdat
reviewed
Mar 16, 2026
Comment on lines
+38
to
+40
| if (customPrompt.trim().isEmpty) { | ||
| throw ArgumentError('Custom prompt cannot be empty'); | ||
| } |
Member
There was a problem hiding this comment.
IMO, some action not need customPrompt
Member
There was a problem hiding this comment.
Why not for me:
- this is just for custom prompt so when you write in the input "Write an email about ..."
- UI button to send message is disabled if there is no content in the input so it does not fix a bug for now
- but it makes no sense to send a empty custom prompt
So better to keep it in case the UI change.
Member
|
About CORS issue:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
#4378
Resolved
Summary by CodeRabbit
New Features
Bug Fixes
Improvements