-
Notifications
You must be signed in to change notification settings - Fork 0
Kh/dev/mail adjustments #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates email template styling and structure, including a comprehensive refactoring of the HTML head/tail sections and modifications to color schemes and font sizes across multiple email components. The changes also add user language detection functionality to personalize emails based on recipient preferences.
Key Changes:
- Modernized email template structure with simplified HTML head and new tail structure
- Updated color scheme from
#465A75to#001B41across multiple components - Added user language detection to send emails in recipient's preferred language
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/templates/email/head.html | Complete rewrite of HTML head section with simplified meta tags and responsive styles |
| lib/templates/email/tail.html | Added closing table tags and Gmail iOS font manipulation prevention |
| lib/templates/email/header.html | Updated logo dimensions and removed explicit width styling |
| lib/templates/email/heading.html | Increased heading font size from 26px to 32px and updated color |
| lib/templates/email/footer.html | Updated font sizes and colors for footer content |
| lib/templates/email/bodyText.html | Changed text color to match new color scheme |
| lib/templates/email/bodyEnd.html | Updated greeting text color |
| lib/templates/email/listItem.html | Replaced icon img tag with placeholder and updated text color |
| lib/templates/email/button.html | Increased button font size from 12px to 15px |
| lib/templates/email/buttonGroup.html | Increased button font size from 12px to 15px |
| lib/EMailTemplate.php | Added user detection logic to determine recipient language preferences |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <td class="heading-title" align="left" > | ||
| <p class="heading" style="padding:0 0 0 0;margin:0;text-align:left;line-height:33px" > | ||
| <font class="heading-font" style="font-family:\'Overpass Regular\', \'Roboto\', Arial, sans-serif;font-size:26px;font-weight:600;color:#0B2A63;line-height:33px" > | ||
| <font class="heading-font" style="font-family:\'Overpass Regular\', \'Roboto\', Arial, sans-serif;font-size:32px;font-weight:600;color:#0b2a63;line-height:33px" > |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The color value case changed from uppercase #0B2A63 to lowercase #0b2a63. For consistency, maintain the same case format for hex color values across all templates. Consider standardizing to lowercase throughout.
| <td class="w-112 minw-112 h-32 minh-32" align="left" valign="middle" height="32" style="min-width:112px;width:112px;min-height:32px;height:32px" > | ||
| <img src="<?= $logoUrl ?>" border="0" title="IONOS" alt="IONOS Logo" class="w-112 h-32" width="112" height="32" style="color:#465A75;margin:0;border:0;font-size:15px;font-family:'Open Sans', 'Google Sans', Arial, sans-serif;line-height:0;display:block;width:112px;height:32px;padding:0" /> | ||
| <td class="w-112 minw-112 h-24 minh-24" align="left" valign="middle" height="24" style="min-width:112px;width:112px;min-height:24px;height:24px" > | ||
| <img src="<?= $logoUrl ?>" border="0" title="IONOS" alt="IONOS Logo" class="h-32" height="24" style="color:#465A75;margin:0;border:0;font-size:15px;font-family:'Open Sans', 'Google Sans', Arial, sans-serif;line-height:0;display:block;height:24px;padding:0" /> |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name h-32 suggests a height of 32px, but the actual height is now 24px. This creates confusion between the class name and the actual styling. Consider updating the class name to h-24 to match the actual height value.
| <img src="<?= $logoUrl ?>" border="0" title="IONOS" alt="IONOS Logo" class="h-32" height="24" style="color:#465A75;margin:0;border:0;font-size:15px;font-family:'Open Sans', 'Google Sans', Arial, sans-serif;line-height:0;display:block;height:24px;padding:0" /> | |
| <img src="<?= $logoUrl ?>" border="0" title="IONOS" alt="IONOS Logo" class="h-24" height="24" style="color:#465A75;margin:0;border:0;font-size:15px;font-family:'Open Sans', 'Google Sans', Arial, sans-serif;line-height:0;display:block;height:24px;padding:0" /> |
fc0443e to
9168f7f
Compare
| private function determineUser(array $data): ?IUser { | ||
| $userManager = \OC::$server->get(IUserManager::class); | ||
|
|
||
| // Priority 1: Try to get recipient by user ID (most direct) | ||
| $userIdKeys = ['userid', 'userId', 'uid']; | ||
| foreach ($userIdKeys as $key) { | ||
| if (isset($data[$key]) && is_string($data[$key])) { | ||
| $user = $userManager->get($data[$key]); | ||
| if ($user instanceof IUser) { | ||
| return $user; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Priority 2: Try to get recipient by email address | ||
| $emailKeys = ['emailAddress', 'newEMailAddress', 'shareWith']; | ||
| foreach ($emailKeys as $key) { | ||
| if (isset($data[$key]) && is_string($data[$key])) { | ||
| $value = $data[$key]; | ||
|
|
||
| // shareWith might be a user ID or email, try as user ID first | ||
| if ($key === 'shareWith') { | ||
| $user = $userManager->get($value); | ||
| if ($user instanceof IUser) { | ||
| return $user; | ||
| } | ||
| } | ||
|
|
||
| // Try to get user by email address | ||
| $users = $userManager->getByEmail($value); | ||
| if (!empty($users)) { | ||
| return reset($users); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Priority 3: Try attendee_name or displayname as user ID or display name search | ||
| $nameKeys = ['attendee_name', 'displayname']; | ||
| foreach ($nameKeys as $key) { | ||
| if (isset($data[$key]) && is_string($data[$key])) { | ||
| $value = $data[$key]; | ||
|
|
||
| // First try as user ID | ||
| $user = $userManager->get($value); | ||
| if ($user instanceof IUser) { | ||
| return $user; | ||
| } | ||
|
|
||
| // Then try as display name search | ||
| $users = $userManager->searchDisplayName($value, 1); | ||
| if (!empty($users)) { | ||
| return reset($users); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. But the whole concept of this file html breaking needs to updated for a proper solution maybe in a later PR.
For now it works fine.
Also, a minor improvement can be made. 'Sharewith' check can be moved in the Priority 1 check, as it is same for the userId/uid get().
And is 3rd priority to find with display_name really required? (Do we have any user without user_id or email which only have their display_name). As it could return some other user too.
Also the email lookup can be made if there is an "@" in data, so the lookup is more faster and won't pass any other data to getByEmail function.
Code snippet:
`$userManager = \OC::$server->get(IUserManager::class);
// Strategy 1: Direct user ID lookup (fastest, most accurate)
$userIdKeys = ['userid', 'userId', 'uid', 'shareWith'];
foreach ($userIdKeys as $key) {
if (isset($data[$key]) && is_string($data[$key]) && !str_contains($data[$key], '@')) {
$user = $userManager->get($data[$key]);
if ($user instanceof IUser) {
return $user;
}
}
}
// Strategy 2: Email address lookup (accurate but slower)
$emailKeys = ['emailAddress', 'newEMailAddress', 'shareWith'];
foreach ($emailKeys as $key) {
if (isset($data[$key]) && is_string($data[$key]) && str_contains($data[$key], '@')) {
$users = $userManager->getByEmail($data[$key]);
if (!empty($users)) {
return reset($users);
}
}
}`
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Arsalanulhaq regarding strat 3 with display_name or attandee_name
Depends on what we are getting as ’data’ and it seems like for example for the Acitivity mail we are not receiving an userid or email address but just the diplay name. I could be wrong but i suspect there will be more Mail types with that problem.
Also shareWith is described as:
Email address of the recipient
Thats why I had it in the strat 2
9168f7f to
282dacb
Compare
282dacb to
1026f2c
Compare
There are two template literals 1. Icon 2. Text As the second one was missing we had two icons and no text in those list items Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
1026f2c to
4ad0f18
Compare
No description provided.