Conversation
|
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 (10)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds read-only support to labels: a nullable Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
labels/lib/extensions/label_extension.dart (1)
32-43:⚠️ Potential issue | 🟠 MajorPreserve
readOnlyincopyWithto avoid dropping state.
copyWithcurrently rebuildsLabelwithoutreadOnly, so copied labels can silently lose read-only protection.🔧 Proposed fix
Label copyWith({ Id? id, KeyWordIdentifier? keyword, String? displayName, HexColor? color, + bool? readOnly, }) { return Label( id: id ?? this.id, displayName: displayName ?? this.displayName, keyword: keyword ?? this.keyword, color: color ?? this.color, + readOnly: readOnly ?? this.readOnly, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@labels/lib/extensions/label_extension.dart` around lines 32 - 43, The copyWith implementation on Label is dropping the readOnly field; update Label.copyWith to accept an optional bool? readOnly parameter and when constructing the new Label pass readOnly: readOnly ?? this.readOnly so the copied instance preserves existing read-only state (and can be overridden when provided). Ensure the parameter name matches the Label.readOnly property and add it to the returned Label constructor call.
🧹 Nitpick comments (1)
lib/features/labels/presentation/label_controller.dart (1)
55-55: Capability reactivity: optional improvement, not a correctness issue.While
_labelsCapabilityis a plain field, the code functions correctly because capability updates are coupled with reactivelabelsupdates. WhencheckLabelSettingStateupdates the capability at line 77, it immediately triggersgetAllLabels(), which updateslabels.value(reactive). TheObxwrapper at line 401 rebuilds whenlabelschanges, soshouldAskReadOnlyis evaluated with the current capability. Converting toRxn<LabelsCapability>would be cleaner design, but the current pattern avoids stale state through coupled updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/labels/presentation/label_controller.dart` at line 55, The field _labelsCapability is currently a plain (nullable) LabelsCapability which relies on coupled updates to avoid staleness; convert it to a reactive Rxn<LabelsCapability> (e.g., Rxn<LabelsCapability> _labelsCapability = Rxn()) and update all usages: set capability via _labelsCapability.value in checkLabelSettingState, read it via _labelsCapability.value where shouldAskReadOnly and any other logic reference it, and ensure any side-effects that previously called getAllLabels() still do so after updating .value so observers (like labels and the Obx at line 401) react properly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@labels/lib/extensions/label_extension.dart`:
- Around line 32-43: The copyWith implementation on Label is dropping the
readOnly field; update Label.copyWith to accept an optional bool? readOnly
parameter and when constructing the new Label pass readOnly: readOnly ??
this.readOnly so the copied instance preserves existing read-only state (and can
be overridden when provided). Ensure the parameter name matches the
Label.readOnly property and add it to the returned Label constructor call.
---
Nitpick comments:
In `@lib/features/labels/presentation/label_controller.dart`:
- Line 55: The field _labelsCapability is currently a plain (nullable)
LabelsCapability which relies on coupled updates to avoid staleness; convert it
to a reactive Rxn<LabelsCapability> (e.g., Rxn<LabelsCapability>
_labelsCapability = Rxn()) and update all usages: set capability via
_labelsCapability.value in checkLabelSettingState, read it via
_labelsCapability.value where shouldAskReadOnly and any other logic reference
it, and ensure any side-effects that previously called getAllLabels() still do
so after updating .value so observers (like labels and the Obx at line 401)
react properly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 524612c8-663b-47ab-8045-8d3e7496384d
📒 Files selected for processing (10)
labels/lib/extensions/label_extension.dartlabels/lib/labels.dartlabels/lib/model/label.dartlabels/lib/model/labels_capability.dartlib/features/home/domain/converter/capability_properties_converter.dartlib/features/home/domain/extensions/session_extensions.dartlib/features/labels/presentation/label_controller.dartlib/features/mailbox/presentation/base_mailbox_view.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.dartlib/features/mailbox/presentation/widgets/labels/label_list_view.dart
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4412. |
dab246
force-pushed
the
enhancement/tf-4383-handle-read-only-labels
branch
from
c1aef32 to
5b4c941
Compare
|
What is the purpose? No long press action is performed when readyOnly = true. |
To notice user that label cannot change |
We haven't performed any action that would require a user notification. |
2 participants
Issue
#4383
Resolved
readOnly=false:demo1.mov
readOnly=true:Screen.Recording.2026-03-25.at.18.34.26.mov
Summary by CodeRabbit
New Features
Chores