feat: Allow pasting multiple recipients at once#2873
feat: Allow pasting multiple recipients at once#2873Elouan1411 wants to merge 9 commits intomainfrom
Conversation
Elouan1411
force-pushed
the
paste-multiple-recipients
branch
from
8272efb to
e23f546
Compare
|
Failed to generate code suggestions for PR |
1 similar comment
|
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
Pull request overview
Adds support for pasting/entering multiple recipients at once in the new-message recipient fields, including bulk validation, recipient-limit handling, and localized pluralized feedback messages.
Changes:
- Intercept paste events in the recipient input to parse and add multiple recipients separated by commas/semicolons/whitespace.
- Add bulk-add logic to recipient chips (including duplicate detection and recipient-limit warnings) and resolve display names via a contact lookup by email.
- Introduce new localized plural strings for bulk-add warnings (duplicates, invalid emails, limit reached).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds new pluralized strings for bulk recipient paste feedback. |
| app/src/main/res/values-sv/strings.xml | Swedish translations for new pluralized bulk-add strings. |
| app/src/main/res/values-pt/strings.xml | Portuguese translations for new pluralized bulk-add strings. |
| app/src/main/res/values-pl/strings.xml | Polish translations (with additional plural quantities) for new bulk-add strings. |
| app/src/main/res/values-nl/strings.xml | Dutch translations for new pluralized bulk-add strings. |
| app/src/main/res/values-nb/strings.xml | Norwegian Bokmål translations for new pluralized bulk-add strings. |
| app/src/main/res/values-it/strings.xml | Italian translations for new pluralized bulk-add strings. |
| app/src/main/res/values-fr/strings.xml | French translations for new pluralized bulk-add strings. |
| app/src/main/res/values-fi/strings.xml | Finnish translations for new pluralized bulk-add strings. |
| app/src/main/res/values-es/strings.xml | Spanish translations for new pluralized bulk-add strings. |
| app/src/main/res/values-el/strings.xml | Greek translations for new pluralized bulk-add strings. |
| app/src/main/res/values-de/strings.xml | German translations for new pluralized bulk-add strings. |
| app/src/main/res/values-da/strings.xml | Danish translations for new pluralized bulk-add strings. |
| app/src/main/java/com/infomaniak/mail/ui/newMessage/RecipientFieldView.kt | Implements multi-recipient parsing, paste interception, bulk chip insertion, and warning snackbar logic. |
| app/src/main/java/com/infomaniak/mail/ui/newMessage/NewMessageViewModel.kt | Exposes merged-contact lookup by email for recipient display-name resolution. |
| app/src/main/java/com/infomaniak/mail/ui/newMessage/NewMessageRecipientFieldsManager.kt | Wires the new email-to-merged-contact callback into recipient fields. |
| app/src/main/java/com/infomaniak/mail/ui/newMessage/ContactChipAdapter.kt | Adds API to insert multiple chips efficiently. |
| app/src/main/java/com/infomaniak/mail/ui/newMessage/BackspaceAwareTextInput.kt | Adds a paste-intercept hook by overriding the paste context-menu action. |
| app/src/main/java/com/infomaniak/mail/data/cache/userInfo/MergedContactController.kt | Adds Realm query to fetch a merged contact by email. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR Reviewer Guide 🔍(Review updated until commit 19609a3)Here are some key observations to aid the review process:
|
|
Failed to generate code suggestions for PR |
Elouan1411
force-pushed
the
paste-multiple-recipients
branch
from
7bb2b39 to
19609a3
Compare
|
Persistent review updated to latest commit 19609a3 |
|
Failed to generate code suggestions for PR |
UI needs improvement (currently displays email instead of name and requires pressing Enter to confirm)
…s and enable automatic chip creation without pressing Enter
Elouan1411
force-pushed
the
paste-multiple-recipients
branch
from
19609a3 to
22bb36e
Compare
FabianDevel
left a comment
FabianDevel
left a comment
There was a problem hiding this comment.
First part of review
| fun addChips(newRecipients: List<Recipient>): Int { | ||
| var added = 0 | ||
| newRecipients.forEach { recipient -> if (recipients.add(recipient)) added++ } | ||
| if (added > 0) notifyItemRangeInserted(itemCount - added, added) | ||
|
|
||
| return added | ||
| } |
There was a problem hiding this comment.
Why do you compute this return value but never use it ?
Also, a more "advanced" kotlin way of doing this could be :
| fun addChips(newRecipients: List<Recipient>): Int { | |
| var added = 0 | |
| newRecipients.forEach { recipient -> if (recipients.add(recipient)) added++ } | |
| if (added > 0) notifyItemRangeInserted(itemCount - added, added) | |
| return added | |
| } | |
| fun addChips(newRecipients: List<Recipient>): Int { | |
| return newRecipients.fold(initial = 0) { added, recipient -> | |
| if (recipients.add(recipient)) added + 1 else added | |
| } | |
| } |
This has the advantadge to scope you local variable, and avoid a var
This can protect a bit your code from error (assigning a value to a var that you shouldn't, using a variable that's living longer than it should etc.)
There was a problem hiding this comment.
I computed this value to align with the addChip function which also provides information on whether it was added. Should I remove it or keep it?
| } | ||
|
|
||
| if (contactChipAdapter.itemCount > MAX_ALLOWED_RECIPIENT) { | ||
| if (contactChipAdapter.itemCount >= MAX_ALLOWED_RECIPIENT) { |
There was a problem hiding this comment.
Why did you add this ?
There was a problem hiding this comment.
I don't know if it's intentional, but I noticed that using only > allows adding one extra element (if MAX_ALLOWED_RECIPIENT = 99, you can add 100 recipients).
Elouan1411
force-pushed
the
paste-multiple-recipients
branch
2 times, most recently
from
239353b to
9bfa10d
Compare
|
No description provided.