Fix: Prevent "Powered by Ghost" from opening two tabs#24623
Conversation
This change ensures that clicking on the Powered By link does not open two tabs.
WalkthroughRemoved the custom onClick handler from the anchor in apps/portal/src/components/common/PoweredBy.js that previously called window.open('https://ghost.org', '_blank'). The anchor now relies on href="https://ghost.org" with target="_blank" and rel="noopener noreferrer" and includes a data-test-powered-by attribute. Added a test in apps/portal/src/tests/portal-links.test.js that renders the portal signup, finds the "powered by ghost" link in the popup, and asserts href, target, rel, and absence of an onClick handler. No other component structure, content, or exports changed. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/portal/src/components/common/PoweredBy.js (1)
14-14: Accessibility: hide decorative SVG from screen readers.Mark the logo as decorative so the link’s accessible name is just “Powered by Ghost.”
- <GhostLogo /> + <GhostLogo aria-hidden="true" focusable="false" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/portal/src/components/common/PoweredBy.js(1 hunks)
🔇 Additional comments (1)
apps/portal/src/components/common/PoweredBy.js (1)
13-16: Fix looks good: removes double navigation and keeps tabnabbing protections.Relying on the anchor’s default behavior with target="_blank" and rel="noopener noreferrer" resolves the two-tab issue and preserves security best practices.
Adds a data-test-powered-by attribute to the external link to simplify E2E targeting.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/portal/src/tests/portal-links.test.js (2)
362-362: Make the rel assertion order-insensitive.Avoid brittleness if token order changes. Assert both tokens are present.
- expect(poweredByLink).toHaveAttribute('rel', 'noopener noreferrer'); + const rel = poweredByLink.getAttribute('rel') || ''; + expect(rel).toContain('noopener'); + expect(rel).toContain('noreferrer');
357-357: Prefer findByRole for async rendering in iframe.Using findByRole reduces flakiness if the link renders slightly after the iframe is ready.
- const poweredByLink = within(popupFrame.contentDocument).getByRole('link', {name: /powered by ghost/i}); + const poweredByLink = await within(popupFrame.contentDocument).findByRole('link', {name: /powered by ghost/i});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/portal/src/tests/portal-links.test.js(1 hunks)
🔇 Additional comments (1)
apps/portal/src/tests/portal-links.test.js (1)
347-355: Good coverage of the PoweredBy link semantics.Asserting presence, href, target, and rel on the link is solid and aligns with the fix.
120d132 to
9bb8bdc
Compare
9bb8bdc to
21af29b
Compare
|
@niranjan-uma-shankar Sorry for the delay and the commit noise - the rebase was a bit messy. Thanks for the submission! |
Summary
In Portal, clicking on "Powered by Ghost", in the overlay that appears in the signup modal, opens ghost.org in two tabs. Example:
Screen.Recording.2025-08-07.at.7.40.18.PM.mov
Root cause:
hrefattribute and anonClickhandler, leading to two tabs for a single click.Fix
onClickhandler.Testing Instructions