🐛 Fixed link selection in automations email editor#29176
Conversation
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run-many -t test:unit -p @tryghost/posts,@tr... |
✅ Succeeded | 2m 20s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 57s | View ↗ |
nx run-many -t lint -p @tryghost/posts,@tryghos... |
✅ Succeeded | 39s | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 8s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-09 12:27:49 UTC
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe email content modal now detects focus inside Koenig portals, keeps background body children inert while the dialog is mounted, and changes Escape and outside-interaction handling so Koenig popups are not dismissed unintentionally. The dialog is rendered non-modally and uses the dialog content node for mounting and cleanup logic. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
🤖 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
`@apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx`:
- Around line 232-251: The inert handling in the email content modal only
snapshots document.body.children once, so body-level portals mounted after the
modal opens can remain interactive behind it. Update the useEffect in
email-content-modal.tsx to keep enforcing inert on newly added body children
while the dialog is open, either by re-running the inert pass when body children
change or by observing document.body; keep the existing dialogContentRef and
dialogPortalWrapper logic intact.
🪄 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: 63c77390-a036-4190-8d46-e2f0e01e35ca
📒 Files selected for processing (1)
apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx
EvanHahn
left a comment
EvanHahn
left a comment
There was a problem hiding this comment.
I could be convinced that this is a good solution, but it feels like the right answer is to fix something about Koenig (and/or our usage of Radix). I know you tried that stuff, so maybe there's no better alternative.
troyciesco
force-pushed
the
NY-1394_atmns-editor-links
branch
2 times, most recently
from
f74bed9 to
0d0dab1
Compare
ref https://linear.app/ghost/issue/NY-1394/ - Koenig portals its link input/toolbar to document.body, but the automations email modal's Radix dialog is modal, so its focus trap couldn't contain those portals — clicks and focus never reliably reached them - attempted a Koenig-side fix (render portals into a container inside the dialog instead), but that surfaced a new focus fight between Radix's trap and Lexical's own focus handling - fixed in Ghost instead: the dialog is non-modal, and the rest of the admin UI is made inert while it's open, so nothing outside the editor can steal focus/clicks and Koenig's body-level portals work fine - inert is also more robust for accessibility than a JS focus trap — it's natively enforced by the browser, so background content is reliably unreachable by keyboard and screen readers regardless of where Koenig portals its UI
ref https://linear.app/ghost/issue/NY-1394/ - while responding to a CodeRabbit review comment on the previous commit, found that the effect making the background admin UI inert while the email editor is open was a silent no-op: dialogContentRef was a plain useRef, but Radix's Portal primitive renders null on its first pass and only mounts the real DOM node one render later (via its own useLayoutEffect, an SSR-safety trick since document isn't available server-side) — so dialogContentRef.current was still null by the time our effect ran, it bailed out immediately, and with an empty dependency array it never got a second chance - verified via devtools: document.querySelectorAll('[inert]') returned nothing before this fix, meaning the background was fully focusable/clickable the entire time the modal was open - fixed by tracking the node as state via a callback ref instead of a plain ref, so the effect re-runs once Radix actually attaches it - this also let us properly re-test CodeRabbit's suggestion of adding a MutationObserver to catch late-mounting body children: with the inert-lock actually working, that suggestion does break the stacked discard-confirm AlertDialog, confirming it shouldn't be applied
troyciesco
force-pushed
the
NY-1394_atmns-editor-links
branch
from
0d0dab1 to
e250faa
Compare
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
2 participants
closes https://linear.app/ghost/issue/NY-1394/
before - link input didn't work, and escape would close the entire editor instead of the link input/list:
after - input works, scroll works, escape works, other koenig cards still work. it's hard to demo but there was also an issue with my original approach where if i just set
modal={false}, then tab could potentially move your control to the body (behind the editor). that's the main reason for this larger fix: