adding email template preview tab - #677
Conversation
Signed-off-by: amrita-a-menon <amrita.menon16@gmail.com>
📝 WalkthroughWalkthroughThe email body editor now provides “Edit Template” and “Preview Email” tabs. Preview mode replaces supported merge tags with hardcoded mock values and displays the resulting email body. ChangesEmail preview editor
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/campaign-builder.html (1)
2100-2127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreview logic is correct and safe; consider regex-based substitution for robustness.
The implementation correctly uses
textContent(line 2119) to render preview content, which prevents XSS from user-entered template text. Tab toggling logic is sound.One fragility: the
replaceAllloop (lines 2116-2118) iterates overMOCK_DATAentries sequentially. If any mock value ever contained a substring matching another tag (e.g.,{{company}}set to"{{firstName}} Inc"), it would be double-replaced. A single-pass regex replace avoids this:♻️ Optional: single-pass regex substitution
let preview = document.getElementById('ep-body').value; - Object.entries(MOCK_DATA).forEach(([tag, value]) => { - preview = preview.replaceAll(tag, value); - }); + preview = preview.replace(/\{\{(\w+)\}\}/g, (match, key) => { + const tag = `{{${key}}}`; + return MOCK_DATA.hasOwnProperty(tag) ? MOCK_DATA[tag] : match; + }); document.getElementById('email-preview-box').textContent = preview;This also leaves unrecognized tags (e.g., custom variables like
{{industry}}that the backend supports perpersonalize_email) visible in the preview rather than silently leaving them, which gives the user a clearer picture of what will and won't be substituted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/campaign-builder.html` around lines 2100 - 2127, Update the preview handler using MOCK_DATA to perform one-pass placeholder substitution instead of sequential replaceAll calls. Build a regex from the MOCK_DATA keys with proper escaping and replace each match via its corresponding value, preserving unrecognized template tags in the rendered preview and the existing textContent assignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/campaign-builder.html`:
- Around line 2100-2127: Update the preview handler using MOCK_DATA to perform
one-pass placeholder substitution instead of sequential replaceAll calls. Build
a regex from the MOCK_DATA keys with proper escaping and replace each match via
its corresponding value, preserving unrecognized template tags in the rendered
preview and the existing textContent assignment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ea379f1a-eb2e-4bb5-94ad-d41be4dd2285
📒 Files selected for processing (1)
frontend/campaign-builder.html
|
@Kuldeeep18 , please do take a look into this PR at your convenience and let me know if there is anything that you would prefer changed. Hope we can get this merged soon. Cheers! |
|
@Kuldeeep18 , just boosting this thread up. Completed this PR 2 weeks ago, so I hope I can get your approval and get this merged as soon as possible. Cheers! |
Pull Request
🔗 Related Issue
Closes #449
📝 Summary of Changes
🏷️ Type of Change
🧪 Testing
Tested locally in the browser.
Steps to test:
📸 Screenshots (if applicable)
✅ Checklist
Summary by CodeRabbit