Added size warning to email preview#13
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a size warning to the email preview modal to alert users when an email exceeds 100kB, which can cause clipping in some email clients. The implementation refactors the existing email size warning system from a two-tier warning (yellow/red) to a single boolean threshold check.
Changes:
- Simplified email size warning service from dual thresholds (90KB yellow, 100KB red) to single 100KB limit
- Added size warning UI to email preview modal and publish flow options
- Updated email size warning component to support yielding data via blocks
- Modified CSS to use yellow color variables for warning indicators
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ghost/admin/app/services/email-size-warning.js | Refactored from warningLevel (red/yellow) to boolean overLimit flag with single 100KB threshold |
| ghost/admin/app/components/editor/email-size-warning.js | Simplified component to track overLimit boolean instead of warningLevel, removed post getter |
| ghost/admin/app/components/editor/email-size-warning.hbs | Added block component support to yield warning state for custom rendering |
| ghost/admin/app/components/editor/modals/preview/email.hbs | Added warning banner in email preview when size exceeds limit |
| ghost/admin/app/components/editor/modals/publish-flow/options.hbs | Added warning box in publish options when email will be sent |
| ghost/admin/app/styles/layouts/preview-email.css | Added styles for warning container in email preview |
| ghost/admin/app/styles/layouts/editor.css | Changed warning icon stroke color to yellow |
| ghost/admin/app/styles/components/publishmenu.css | Added transition animation for warning appearance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .gh-email-preview-clip-description { | ||
| font-size: 1.3rem; | ||
| text-wrap: pretty; | ||
| color: var(--middarkgrey) |
There was a problem hiding this comment.
Missing semicolon at end of CSS declaration. While this may work in most browsers, it's best practice to always include the semicolon for consistency and to prevent potential issues.
| <div class="gh-email-preview-clip-container"> | ||
| {{svg-jar "email-warning"}} | ||
| <div> | ||
| <div class="gh-email-preview-clip-title">This email is <span class="yellow-d1">{{emailSizeKb}}kB</span></div> |
There was a problem hiding this comment.
Inconsistent styling approach. This line uses a yellow-d1 class directly on the span, while line 45 in options.hbs uses the same pattern. Consider if this should match the popup implementation which uses inline style or CSS variable for consistency across the codebase.
| </div> | ||
| {{else}} | ||
| <span {{did-update this.checkEmailSize @post.updatedAtUTC}} class="gh-editor-email-size-warning-container"> | ||
| <span class="gh-editor-email-size-warning gh-editor-email-size-warning--{{this.overLimit}}" data-warning-active={{if this.overLimit "true" "false"}}> |
There was a problem hiding this comment.
The class gh-editor-email-size-warning--{{this.overLimit}} will produce gh-editor-email-size-warning--true or gh-editor-email-size-warning--false. This differs from the previous implementation which used specific warning levels like 'yellow' or 'red'. Ensure corresponding CSS exists for these boolean-based class names, or consider using a more semantic class name.
Benchmark PR from qodo-benchmark#104