🐛 Fixed Preview button being unavailable on mobile in the editor#28601
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request makes three independent changes across RSS feed generation, the post preview modal, and mobile editor styling. In the RSS feed service ( Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ghost/core/core/frontend/services/rss/generate-feed.js (1)
20-20:⚠️ Potential issue | 🟠 MajorUpgrade cheerio from 0.22.0 to latest (1.2.0).
Cheerio 0.22.0 was released in August 2016 and is now nearly a decade old. This version relies on outdated transitive dependencies with known security vulnerabilities. The official security policy states that only the latest releases receive security updates. Upgrade to version 1.2.0 (latest as of January 2026) to address vulnerable dependencies and ensure compatibility with modern Node.js environments.
🤖 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 `@ghost/core/core/frontend/services/rss/generate-feed.js` at line 20, The cheerio dependency needs to be upgraded to address security vulnerabilities in its outdated transitive dependencies. Locate the cheerio dependency declaration in the package.json file and update the version from 0.22.0 to 1.2.0 (the latest version as of January 2026), then run npm install to install the updated package. The require statement for cheerio in generate-feed.js does not require code changes as it will automatically use the updated version.
🧹 Nitpick comments (1)
ghost/core/test/unit/frontend/services/rss/generate-feed.test.js (1)
264-295: ⚡ Quick winAdd test coverage for video cards without thumbnails.
The current test provides both
thumbnailSrcandcustomThumbnailSrc, ensuring a poster is always available. Consider adding a test case where neither thumbnail is provided to verify the implementation handles missing posters gracefully (this would catch the undefined poster issue flagged ingenerate-feed.js).📝 Suggested test case
it('handles video cards without thumbnails gracefully', async function () { const html = callRenderer('video', { src: '/content/x.mp4', mimeType: 'video/mp4', width: 200, height: 100, duration: 60 // No thumbnailSrc or customThumbnailSrc }).html; data.posts = [Object.assign({}, posts[1], {html})]; const xmlData = await generateFeed(baseUrl, data); assertExists(xmlData); const content = getEncodedContent(xmlData); const video = content.match(/<video[^>]*>/); assertExists(video); // Should have controls but no poster="undefined" assert.match(video[0], /controls/); assert.doesNotMatch(video[0], /poster="undefined"/); });🤖 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 `@ghost/core/test/unit/frontend/services/rss/generate-feed.test.js` around lines 264 - 295, Add a new test case after the existing video test to verify that video cards without thumbnails are handled gracefully. Create a test that calls callRenderer with video configuration (src, mimeType, width, height, duration) but omits both thumbnailSrc and customThumbnailSrc parameters. Verify that the resulting video element in the generated feed contains the controls attribute but does not contain a poster="undefined" attribute, ensuring the implementation gracefully handles missing poster images without generating invalid HTML attributes.
🤖 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.
Inline comments:
In `@ghost/core/core/frontend/services/rss/generate-feed.js`:
- Around line 38-40: The issue is that when neither `data-kg-custom-thumbnail`
nor `data-kg-thumbnail` attributes exist, the `videoPoster` variable becomes
`undefined`, and passing this to `video.attr('poster', videoPoster)` causes
Cheerio 0.22.0 to set the poster attribute to the string `"undefined"` in the
RSS feed. Guard against this by conditionally setting the poster attribute on
the `video` element only when `videoPoster` is defined and has a truthy value,
preventing the undefined value from being written to the output.
---
Outside diff comments:
In `@ghost/core/core/frontend/services/rss/generate-feed.js`:
- Line 20: The cheerio dependency needs to be upgraded to address security
vulnerabilities in its outdated transitive dependencies. Locate the cheerio
dependency declaration in the package.json file and update the version from
0.22.0 to 1.2.0 (the latest version as of January 2026), then run npm install to
install the updated package. The require statement for cheerio in
generate-feed.js does not require code changes as it will automatically use the
updated version.
---
Nitpick comments:
In `@ghost/core/test/unit/frontend/services/rss/generate-feed.test.js`:
- Around line 264-295: Add a new test case after the existing video test to
verify that video cards without thumbnails are handled gracefully. Create a test
that calls callRenderer with video configuration (src, mimeType, width, height,
duration) but omits both thumbnailSrc and customThumbnailSrc parameters. Verify
that the resulting video element in the generated feed contains the controls
attribute but does not contain a poster="undefined" attribute, ensuring the
implementation gracefully handles missing poster images without generating
invalid HTML attributes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6bd29735-303f-4b07-b34a-cb0353f00b59
📒 Files selected for processing (3)
ghost/admin/app/styles/layouts/editor.cssghost/core/core/frontend/services/rss/generate-feed.jsghost/core/test/unit/frontend/services/rss/generate-feed.test.js
| const videoPoster = htmlContent(card).attr('data-kg-custom-thumbnail') || htmlContent(card).attr('data-kg-thumbnail'); | ||
| const video = htmlContent(card).find('.kg-video-card video'); | ||
| video.attr('poster', videoPoster); |
There was a problem hiding this comment.
Guard against undefined poster attribute.
When a video card has neither data-kg-custom-thumbnail nor data-kg-thumbnail, videoPoster will be undefined. Setting .attr('poster', undefined) in Cheerio 0.22.0 likely converts it to the string "undefined", producing poster="undefined" in the RSS feed.
🛡️ Proposed fix to conditionally set poster
const videoPoster = htmlContent(card).attr('data-kg-custom-thumbnail') || htmlContent(card).attr('data-kg-thumbnail');
const video = htmlContent(card).find('.kg-video-card video');
-video.attr('poster', videoPoster);
+if (videoPoster) {
+ video.attr('poster', videoPoster);
+}
video.attr('controls', '');📝 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.
| const videoPoster = htmlContent(card).attr('data-kg-custom-thumbnail') || htmlContent(card).attr('data-kg-thumbnail'); | |
| const video = htmlContent(card).find('.kg-video-card video'); | |
| video.attr('poster', videoPoster); | |
| const videoPoster = htmlContent(card).attr('data-kg-custom-thumbnail') || htmlContent(card).attr('data-kg-thumbnail'); | |
| const video = htmlContent(card).find('.kg-video-card video'); | |
| if (videoPoster) { | |
| video.attr('poster', videoPoster); | |
| } |
🤖 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 `@ghost/core/core/frontend/services/rss/generate-feed.js` around lines 38 - 40,
The issue is that when neither `data-kg-custom-thumbnail` nor
`data-kg-thumbnail` attributes exist, the `videoPoster` variable becomes
`undefined`, and passing this to `video.attr('poster', videoPoster)` causes
Cheerio 0.22.0 to set the poster attribute to the string `"undefined"` in the
RSS feed. Guard against this by conditionally setting the poster attribute on
the `video` element only when `videoPoster` is defined and has a truthy value,
preventing the undefined value from being written to the output.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ghost/admin/app/styles/layouts/post-preview.css`:
- Line 79: Change the CSS keyword in the fill property from `currentColor` to
lowercase `currentcolor` to comply with Stylelint's keyword casing requirements.
The fill property currently uses mixed-case keyword casing which causes a lint
failure; update it to use all lowercase letters to match the expected style
convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78efdb77-d062-44b9-928f-8371b13c8221
📒 Files selected for processing (1)
ghost/admin/app/styles/layouts/post-preview.css
no ref
- the editor's mobile bottom bar already renders the Preview button, but a blanket `.gh-editor-preview-trigger { display: none }` rule below 500px hid it everywhere, leaving drafts with no way to preview on a phone
- scoped the un-hide to the mobile menu only, so the desktop header and preview-modal header are unaffected
- matches the existing two-button mobile layout already shipped for published posts (Update + Unpublish)
no ref - the preview modal header packed the title, Web/Email tabs, desktop/mobile size toggle, share, Close and Publish into one row with the side groups absolutely positioned, so on phones the centered controls slid under the action group and overlapped - below 640px the header now lays out in flow (no absolute positioning) so the groups can't overlap, and the desktop/mobile size toggle is hidden since it's meaningless on a phone - the Close button shares .gh-editor-preview-trigger, which is hidden below 500px for the editor toolbar; revealed it for the modal (otherwise the preview couldn't be dismissed on a phone) and shows it as a compact X to save space
no ref - close.svg has no fill of its own, so the Close icon rendered black and was invisible on the dark (theme-flipped) preview header; fill it with currentColor so it tracks the button text colour like the share icon does - reordered the action group on mobile so Close sits ahead of Share, anchoring Share directly to the left of Publish
no ref - reverted the mobile-only X back to the "Close" text button to match the desktop modal and drop the heavy bare-icon look - dropped the redundant "Preview" title on phones so the Close, share and Publish controls fit comfortably down to the narrowest widths
no ref - Close at the left edge of the action cluster read as floating mid-header; placing it after Publish anchors it to the corner while keeping share directly left of Publish
9larsons
force-pushed
the
mobile-editor-preview
branch
from
b3809ae to
692f712
Compare
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost-admin:test |
✅ Succeeded | 2m 45s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 2m | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 5s | View ↗ |
nx run-many -t lint -p ghost-admin |
✅ Succeeded | 18s | View ↗ |
nx run-many --target=build --projects=@tryghost... |
✅ Succeeded | <1s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-15 17:17:26 UTC
1 participant
Why
Mobile users can't preview a draft. The editor's mobile bottom bar already renders a Preview button, but a blanket
.gh-editor-preview-trigger { display: none }rule below 500px hid it everywhere — so on a phone a draft has no preview path at all. The only post link that survives on mobile is the "Published →" link, which exists only after publishing (this is what the "can only preview after hitting publish" report was actually seeing).What
1. Show Preview on mobile. Scopes the un-hide to the mobile menu (
.gh-editor-mobile-menu .gh-editor-preview-trigger) so the desktop header and the publish-flow preview trigger are unaffected. This matches the two-button layout already shipped in that bar for published posts (Update + Unpublish), so it adds no clutter. Preview only renders for drafts (post.isDraft), exactly the reported gap.2. Fix the preview modal header on mobile. Opening the modal on a phone surfaced several issues, now fixed:
.gh-editor-preview-trigger, so the same below-500px rule hid it — meaning the preview couldn't be dismissed on a phone at all. It's now revealed for the modal, kept as the same "Close" text button used on desktop, and moved to the far-right corner so it isn't stranded mid-header. Share sits directly left of Publish.Notes
Recording