New email design #61
New email design #61
Conversation
WalkthroughA new HTML email template Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
templates/email/proposal-created-html2.ejs (2)
122-122: Remove empty CSS rulesets.The following empty rulesets can be removed to improve code maintainability:
.es-p-default {}.es-header-body p {}.es-content-body p {}.es-footer-body p {}.es-infoblock p {}Also applies to: 163-169
464-467: Improve image accessibility with better alt text.The alt text for images should be more descriptive to improve accessibility:
- "Logo" could be "Kleros Reality Logo"
- Empty alt text should be replaced with meaningful descriptions
- Social media icons should have consistent alt text format
Also applies to: 699-702, 757-759, 765-769, 775-779
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/email/proposal-created-html2.ejs(1 hunks)
🔇 Additional comments (2)
templates/email/proposal-created-html2.ejs (2)
1-15: Well-structured HTML email template with proper meta tags.
The document structure follows email HTML best practices with appropriate DOCTYPE, meta tags for responsiveness, and email client compatibility declarations.
11-11: Verify required EJS variables documentation.
The template uses several EJS variables. Please ensure these are documented in the README or relevant documentation:
notification.space.ensgetRealityQuestionLink(notification)getSnapshotProposalLink(notification)notification.event.txHashunsubscribeEmail
Also applies to: 532-533, 541-541, 556-556, 571-571, 725-725
✅ Verification successful
Let me check if these variables are documented elsewhere in the codebase, such as in documentation files or in the template's directory.
Let me check if there's any type definition or interface file that might document these variables' structure.
Template variables are well-defined in the codebase
The EJS variables used in the template are properly defined and documented in the codebase:
notification.space.ensis part of theSpaceinterface in the notification type definitiongetRealityQuestionLink(notification)andgetSnapshotProposalLink(notification)are helper functions defined insrc/utils/links.tsnotification.event.txHashis part of bothProposalNotificationEventandAnswerNotificationEventtypesunsubscribeEmailis used consistently across email templates
Additionally, there are test templates (templates/test/*.ejs) that serve as documentation by example, showing the expected structure of all variables.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if variables are documented
echo "Searching for documentation of required EJS variables..."
rg -l "notification\.space\.ens|getRealityQuestionLink|getSnapshotProposalLink|notification\.event\.txHash|unsubscribeEmail" "README.md" || echo "Variables not found in README.md"
Length of output: 334
Script:
#!/bin/bash
# Search for documentation files and check template directory
echo "Searching for documentation files..."
fd -e md -e txt -e doc
echo -e "\nSearching for variable documentation in all files..."
rg -A 2 "notification\.space\.ens|getRealityQuestionLink|getSnapshotProposalLink|notification\.event\.txHash|unsubscribeEmail" --type-add 'docs:*.{md,txt,doc}' --type-add 'template:*.ejs' -t docs -t template
Length of output: 14616
Script:
#!/bin/bash
# Search for type definitions or interfaces
echo "Searching for type definitions..."
rg -A 5 "interface.*Notification|type.*Notification|notification:" --type ts
echo -e "\nSearching for helper function definitions..."
ast-grep --pattern 'function $_(notification) {
$$$
}'
Length of output: 10017
| td .es-button-border-1685481741719:hover { | ||
| background: #7BCBFF !important; | ||
| border-style: solid solid solid solid !important; | ||
| border-color: #aNaNaN #aNaNaN #aNaNaN #aNaNaN !important; |
There was a problem hiding this comment.
Fix invalid color values in CSS.
The border-color property contains invalid color values using 'aNaNaN'. This will not work as expected.
Apply this fix to the affected lines:
-border-color: #aNaNaN #aNaNaN #aNaNaN #aNaNaN !important;
+border-color: transparent !important;Also applies to: 99-99, 110-110
| <a target="_blank" | ||
| href="https://twitter.com/kleros_io" | ||
| style="mso-line-height-rule:exactly;text-decoration:underline;color:#FFFFFF;font-size:14px"><img |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security attributes to external links.
External links should include rel="noopener noreferrer" to prevent potential security issues:
Apply this change to social media links:
-<a target="_blank" href="https://twitter.com/kleros_io"
+<a target="_blank" rel="noopener noreferrer" href="https://twitter.com/kleros_io"Also applies to: 763-765, 772-774
| <a href="<%- unsubscribeEmail %>" | ||
| target="_blank" | ||
| style="mso-line-height-rule:exactly;text-decoration:none;color:#ccbfdd;font-size:12px">Unsubscribe</a> | ||
| </p> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve unsubscribe link visibility.
The unsubscribe link's color (#ccbfdd) has low contrast against the background. Consider using a more visible color to ensure it meets accessibility standards.
Apply this change:
-style="mso-line-height-rule:exactly;text-decoration:none;color:#ccbfdd;font-size:12px"
+style="mso-line-height-rule:exactly;text-decoration:underline;color:#ffffff;font-size:12px"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="<%- unsubscribeEmail %>" | |
| target="_blank" | |
| style="mso-line-height-rule:exactly;text-decoration:none;color:#ccbfdd;font-size:12px">Unsubscribe</a> | |
| </p> | |
| <a href="<%- unsubscribeEmail %>" | |
| target="_blank" | |
| style="mso-line-height-rule:exactly;text-decoration:underline;color:#ffffff;font-size:12px">Unsubscribe</a> | |
| </p> |
New Reality Question
The template variables are the same as the existing template.
Live preview
Test inputs
New Answer
TODO once we approve the other template.
Summary by CodeRabbit