Fixed transistor renderer preview behavior#26790
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughPreview rendering now replaces Transistor iframe blocks with a static placeholder via an exported 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/core/server/services/koenig/node-renderers/transistor-renderer.js (1)
16-22: Consider using CSS instead of deprecatedframeborderattribute.The
frameborderattribute is deprecated in HTML5. While it still works for backwards compatibility, you could use CSSborder: noneon the iframe for a more standards-compliant approach.♻️ Proposed refactor using inline style
function setIframeAttributes(iframe) { iframe.setAttribute('width', '100%'); iframe.setAttribute('height', '370'); - iframe.setAttribute('frameborder', 'no'); iframe.setAttribute('scrolling', 'no'); iframe.setAttribute('seamless', ''); + iframe.setAttribute('style', 'border: none;'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/core/server/services/koenig/node-renderers/transistor-renderer.js` around lines 16 - 22, In setIframeAttributes, stop using the deprecated frameborder attribute and instead apply CSS to the iframe (e.g., set style.border = 'none' or append to iframe.style.cssText) so the iframe has no border; update the function (referencing setIframeAttributes and the iframe parameter) to remove iframe.setAttribute('frameborder', 'no') and add the equivalent style-based rule, and consider using CSS for scrolling behavior too rather than the scrolling attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@ghost/core/core/server/services/koenig/node-renderers/transistor-renderer.js`:
- Around line 16-22: In setIframeAttributes, stop using the deprecated
frameborder attribute and instead apply CSS to the iframe (e.g., set
style.border = 'none' or append to iframe.style.cssText) so the iframe has no
border; update the function (referencing setIframeAttributes and the iframe
parameter) to remove iframe.setAttribute('frameborder', 'no') and add the
equivalent style-based rule, and consider using CSS for scrolling behavior too
rather than the scrolling attribute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39940f4c-cce4-46aa-a8cb-48c5e54f4eb2
📒 Files selected for processing (3)
ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.jsghost/core/core/server/services/koenig/node-renderers/transistor-renderer.jsghost/core/test/unit/server/services/koenig/node-renderers/transistor-renderer.test.js
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js`:
- Around line 121-126: The preview branch that replaces attrs.html when
frame.isPreview uses _buildTransistorPlaceholder() but does not refresh derived
fields, leaving attrs.plaintext and attrs.excerpt stale; after you replace
attrs.html, regenerate the derived text fields the same way other HTML-rewrite
branches do (i.e. invoke the module/helper used elsewhere to compute plaintext
and excerpt from the new attrs.html and assign the results back to
attrs.plaintext and attrs.excerpt) so preview responses return up-to-date
derived values.
- Around line 122-124: The current regex in the attrs.html replacement for
Transistor embeds is too strict about tag adjacency and should allow arbitrary
whitespace/newlines between </iframe>, <script> and <noscript> (e.g. use \s* or
[\s\S]*? between those serialized tags) so jsdom-serialized markup matches;
after performing the replacement (the call that assigns attrs.html =
attrs.html.replace(..., _buildTransistorPlaceholder())), call
_updateTextAttrs(attrs) just like the gated block at line 109 to refresh
attrs.plaintext and attrs.excerpt so derived text fields stay in sync with the
modified HTML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 460be0c8-dd6b-4893-9e77-2aa764c22d3e
📒 Files selected for processing (1)
ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js
ref https://linear.app/ghost/issue/NY-1150/
ref https://linear.app/ghost/issue/NY-1151/