Conversation
Reviewer's GuideAdds a complete HTML email template for user mention notifications, utilizing front-matter metadata, dynamic placeholders, a responsive table-based layout with inline styling, a CTA button, and a social media-enabled footer. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @qinluhe - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `email_template/src/templates/notification_template_with_avatar.html:36` </location>
<code_context>
+ >
+ <tr>
+ <td align="center" style="padding-bottom: 24px">
+ <img
+ src="{{ cdnBaseUrl }}images/appflowy.png"
+ width="64"
+ height="64"
</code_context>
<issue_to_address>
The avatar image is hardcoded and does not use a user-specific avatar.
Consider replacing the static image source with a dynamic variable like {{ userAvatarUrl }} to display the correct user's avatar.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
src="{{ cdnBaseUrl }}images/appflowy.png"
=======
src="{{ userAvatarUrl }}"
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `email_template/src/templates/notification_template_with_avatar.html:183` </location>
<code_context>
+ />
+ </a>
+ <a
+ href="https://x.com/appflowy"
+ style="
+text-decoration: none;
+ display: inline-block;
+ margin: 0 10px 0 0;
+ "
+ >
+ <img
+ src="{{ cdnBaseUrl }}images/twitter.png"
+ width="20"
+ height="20"
</code_context>
<issue_to_address>
The icon for X (formerly Twitter) uses a 'twitter.png' image.
Consider updating the icon to match X's current branding to avoid confusion.
Suggested implementation:
```
<img
src="{{ cdnBaseUrl }}images/x.png"
width="20"
height="20"
```
```
src="{{ cdnBaseUrl }}images/x.png"
width="20"
height="20"
```
```
height="20"
alt="X"
```
Make sure to add the new `x.png` icon to your CDN at the path `images/x.png` to ensure the image loads correctly.
</issue_to_address>
### Comment 3
<location> `email_template/src/templates/notification_template_with_avatar.html:24` </location>
<code_context>
+ color: #000000;
+ "
+ >
+ <tr>
+ <td align="center" valign="top">
+ <table
+ width="600"
</code_context>
<issue_to_address>
The outer table uses a fixed width of 600px, which may not be mobile-friendly.
Consider using width: 100% with a max-width to enhance mobile compatibility and prevent rendering issues on smaller screens.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
<table
width="600"
border="0"
cellpadding="0"
cellspacing="0"
bgcolor="#ffffff"
style="border-radius: 12px; padding: 32px; margin: 0 auto"
>
=======
<table
width="100%"
border="0"
cellpadding="0"
cellspacing="0"
bgcolor="#ffffff"
style="border-radius: 12px; padding: 32px; margin: 0 auto; max-width: 600px;"
>
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <tr> | ||
| <td align="center" style="padding-bottom: 24px"> | ||
| <img | ||
| src="{{ cdnBaseUrl }}images/appflowy.png" |
There was a problem hiding this comment.
suggestion: The avatar image is hardcoded and does not use a user-specific avatar.
Consider replacing the static image source with a dynamic variable like {{ userAvatarUrl }} to display the correct user's avatar.
| src="{{ cdnBaseUrl }}images/appflowy.png" | |
| src="{{ userAvatarUrl }}" |
| href="https://x.com/appflowy" | ||
| style=" | ||
| text-decoration: none; | ||
| display: inline-block; | ||
| margin: 0 10px 0 0; | ||
| " | ||
| > | ||
| <img | ||
| src="{{ cdnBaseUrl }}images/twitter.png" |
There was a problem hiding this comment.
suggestion: The icon for X (formerly Twitter) uses a 'twitter.png' image.
Consider updating the icon to match X's current branding to avoid confusion.
Suggested implementation:
<img
src="{{ cdnBaseUrl }}images/x.png"
width="20"
height="20"
src="{{ cdnBaseUrl }}images/x.png"
width="20"
height="20"
height="20"
alt="X"
Make sure to add the new x.png icon to your CDN at the path images/x.png to ensure the image loads correctly.
| <table | ||
| width="600" | ||
| border="0" | ||
| cellpadding="0" | ||
| cellspacing="0" | ||
| bgcolor="#ffffff" | ||
| style="border-radius: 12px; padding: 32px; margin: 0 auto" | ||
| > |
There was a problem hiding this comment.
suggestion: The outer table uses a fixed width of 600px, which may not be mobile-friendly.
Consider using width: 100% with a max-width to enhance mobile compatibility and prevent rendering issues on smaller screens.
| <table | |
| width="600" | |
| border="0" | |
| cellpadding="0" | |
| cellspacing="0" | |
| bgcolor="#ffffff" | |
| style="border-radius: 12px; padding: 32px; margin: 0 auto" | |
| > | |
| <table | |
| width="100%" | |
| border="0" | |
| cellpadding="0" | |
| cellspacing="0" | |
| bgcolor="#ffffff" | |
| style="border-radius: 12px; padding: 32px; margin: 0 auto; max-width: 600px;" | |
| > |
Summary by Sourcery
New Features: