🐛 Fixed email preview scrollbar and keyboard accessibility #25334
Conversation
WalkthroughReplaces CSS that hid scrollbars with Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ghost/admin/tests/acceptance/editor/post-email-preview-test.js (1)
119-137: Consider clarifying test scope in the title.The test thoroughly validates the focus behavior prerequisites for keyboard scrolling (iframe focusability, body focus, tabindex attributes). However, the test title mentions "scroll" but doesn't actually test scrolling behavior (e.g., simulating Arrow keys and verifying scroll position changes).
Consider either:
- Renaming to something like "allows keyboard users to focus the email preview for keyboard navigation"
- Adding assertions that simulate keyboard events and verify scrolling occurs
That said, testing actual scroll behavior may be complex and potentially fragile. The current test validates the critical prerequisites for keyboard scrolling to work, which may be sufficient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ghost/admin/app/components/editor/modals/preview/email.hbs(1 hunks)ghost/admin/app/components/editor/modals/preview/email.js(3 hunks)ghost/admin/tests/acceptance/editor/post-email-preview-test.js(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-11T14:18:31.752Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24626
File: apps/portal/src/components/pages/AccountHomePage/components/AccountFooter.js:15-15
Timestamp: 2025-08-11T14:18:31.752Z
Learning: In Ghost, Portal components render within iframes while admin components (like gh-member-details.hbs, dashboard/charts/recents.hbs, and posts/post-activity-feed.hbs) do not render in iframes. Therefore, iframe-related navigation issues only affect Portal components.
Applied to files:
ghost/admin/app/components/editor/modals/preview/email.hbs
📚 Learning: 2025-10-15T07:53:49.814Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Use Vitest + Testing Library + jsdom for tests; prioritize focused unit tests for hooks/utils/logic-heavy parts
Applied to files:
ghost/admin/tests/acceptance/editor/post-email-preview-test.js
🧬 Code graph analysis (1)
ghost/admin/app/components/editor/modals/preview/email.js (1)
ghost/admin/tests/acceptance/editor/post-email-preview-test.js (1)
iframe(123-123)
🔇 Additional comments (6)
ghost/admin/app/components/editor/modals/preview/email.hbs (2)
94-94: LGTM! Good accessibility practice.Adding
tabindex="0"correctly makes the iframe keyboard-focusable, allowing users to reach the email preview content via Tab navigation.
98-98: LGTM! Correct event choice for focus handling.Using
focusin(which bubbles) is the right choice for iframe focus detection. This works in tandem withtabindex="0"to enable keyboard navigation into the preview.ghost/admin/app/components/editor/modals/preview/email.js (3)
10-14: LGTM! Correct CSS for scrollbar visibility.Using
overflow-y: autoappropriately shows the vertical scrollbar when content overflows and hides it when content fits, directly addressing the issue. This is preferable tooverflow-y: scroll, which would always show a scrollbar track.
79-106: LGTM! Robust focus management with proper fallbacks.The implementation correctly:
- Sets
tabindex="-1"on the iframe body for programmatic focus (not tab-reachable)- Uses
preventScroll: trueto avoid scrolling the outer page when focusing the iframe- Provides fallbacks for older browsers that don't support
preventScroll- Handles edge cases where
bodymight not be available- Gracefully degrades to window focus on errors
The nested try-catch structure appropriately handles different failure scenarios.
183-183: LGTM! Comment accurately reflects the CSS behavior.The updated comment better describes the purpose of the injected CSS without prescribing specific implementation details.
ghost/admin/tests/acceptance/editor/post-email-preview-test.js (1)
2-2: LGTM! Necessary test helper imports.The added imports (
triggerEvent,waitFor,waitUntil) are all used in the new keyboard navigation test and are standard Ember testing helpers.
fixes TryGhost#25304 Users with two-button mice or keyboard-only workflows could not scroll email previews effectively. The iframe lacked a visible scrollbar and was not keyboard-accessible, creating barriers for accessibility-focused users and those using non-wheel input devices. This fix addresses immediate user pain reported in Windows 11 environments where visible scrollbars are expected UI patterns.
abdultalha0862
force-pushed
the
fix/25304-email-preview-scrollbar
branch
from
1213fb6 to
efa651b
Compare
1 participant
Description
Brief summary of changes:
Fixed email preview modal to display a visible scrollbar and support full keyboard navigation. Users can now scroll email previews using drag-and-drop on the scrollbar, keyboard navigation (arrows/Page Up/Down), or the mouse wheel.
Related issue(s):
Fixes #25304
Type of change:
Testing
How has this been tested?
Test environment:
Manual Testing Performed
Keyboard Navigation:
Mouse/Trackpad:
Automated Test Results
Added acceptance test:
" allows keyboard users to focus and scroll the email preview" that validates:
tabindex="0"tabindex="-1"for keyboard eventsScreenshots
Before:
After:
Issue_After_Fix_25304.mp4
Checklist
Code Quality
Testing
Documentation
Ghost-Specific
Additional Notes
Implementation Details
1. CSS Changes (
email.js):``JavaScriptt
// Before
const INJECTED_CSS =
html::-webkit-scrollbar { display: none; width: 0; background: transparent } html { scrollbar-width: none; };// After
const INJECTED_CSS =
html, body { overflow-y: auto; };Note
Enable visible scrolling and keyboard navigation in the email preview by injecting overflow CSS and focusing the iframe body; adds acceptance test.
editor/modals/preview/email.hbs,email.js):tabindex="0"andfocusinhandler to set focus on iframebody(addstabindex="-1"fallback tocontentWindow.focus()).html, body { overflow-y: auto; }for visible scrolling inside the iframe.post-email-preview-test.js):bodyreceives focus withtabindex="-1".Written by Cursor Bugbot for commit efa651b. This will update automatically on new commits. Configure here.