Newsletter: signup on the About page, and a way back from the confirmation - #1552
Conversation
…om it The form only ever lived in the rail, so an instance offered the digest only if its template rendered one. Four of the nine do not: journal, reader and terminal drop the sidebar seam in their own Shell, and gallery replaces the seam with a sidebar that never mounted the form. Readers of those blogs had no way to subscribe at all while their owner saw the feature switched on in the editor. Every instance has an About page, in both blog and community mode, so the form goes there, mounted in AboutPage itself rather than inside either variant: both return early while their account or community query is loading or has failed, and the signup depends on neither. To keep it to one form per page, the rail stands down on /about. That rule is route based rather than theme based on purpose: whether the rail is rendered at all is each template's own decision, so a theme based rule would have to track nine structures, and gallery already shows the two are not the same question. The placement only changes the frame. The rules, the request and the states are identical, and on the page the heading is an h2 rather than the rail's h3, since the About page opens with an h1 and the rail has nothing above it. The confirmation is no longer a dead end (#1546). A typo gets the same 2xx as a real address, because double opt-in means the service cannot tell them apart, so a reader who mistyped was left waiting for mail that would never arrive with no control on screen and only a page reload to escape. There is a way back now, and focus follows it: the submit button is removed on success, which dropped focus to the document body and sent the next Tab to the top of the page. Focus moves only when it was ours to move, so a reader who clicked elsewhere while the request was in flight keeps their place.
PR Summary by QodoAdd About-page newsletter signup and recoverable confirmation
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1. Tests assert translated English
|
Greptile SummaryThe PR adds the newsletter signup to every About page, suppresses duplicate sidebar placement on that route, and provides a focus-managed path back from the subscription confirmation.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/self-hosted/src/features/blog/components/about-page.tsx | Mounts the page newsletter and centralizes blog and community About outcomes in a shared heading frame. |
| apps/self-hosted/src/features/blog/components/newsletter-signup.tsx | Adds page/sidebar presentation variants, confirmation reset behavior, and guarded focus restoration. |
| apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx | Suppresses the sidebar newsletter on the About route to retain one form per page. |
| apps/self-hosted/src/features/blog/utils/newsletter-signup-target.ts | Adds a normalized pathname predicate for deciding sidebar newsletter visibility. |
| apps/self-hosted/src/features/blog/components/about-page-structure.test.ts | Adds source-level guards for newsletter placement and consistent heading-frame ownership. |
Reviews (3): Last reviewed commit: "Guard the About page mount, and type the..." | Re-trigger Greptile
|
Warning Review limit reached
Next review available in: 44 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe newsletter signup now supports page and sidebar placements, restores focus during state changes, and lets readers enter another address after submission. The About page always renders the page placement, while sidebars hide the signup on ChangesNewsletter signup experience
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR adds newsletter signup access on the About page and a focused path back from confirmation, with the reported tests and build checks passing. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Reader
participant AboutPage
participant NewsletterSignup
participant BlogSidebar
participant sidebarShowsNewsletter
Reader->>AboutPage: open /about
AboutPage->>NewsletterSignup: render placement="page"
BlogSidebar->>sidebarShowsNewsletter: check pathname
sidebarShowsNewsletter-->>BlogSidebar: hide signup
Reader->>NewsletterSignup: submit email
NewsletterSignup-->>Reader: show confirmation and reset control
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Code Review by Qodo
1.
|
Review finding on the page mount: the signup section renders its h2 whatever the account or community query is doing, while both About variants dropped their h1 in the loading and failure branches and returned a bare line of text. It only bites on one template, but it does bite. Three of the four shells carry a masthead h1 of their own, DefaultShell through BlogNavigation and journal and terminal directly, so the document still opens at h1 there. The reader shell renders its title in a span, so on that template a loading or failed About page started at h2. Both variants now return through one frame that owns the heading, and the identity it needs (the handle, the community id, the avatar) comes from config rather than from the request, so a loading or failed page is still a titled, recognisable page instead of the word "loading" on its own. Guarded in the source, in the same shape as failure-states.test.ts next door, because the branch that regresses is a loading state behind a query: every return in either variant must be the frame, and the frame holds the page's only h1.
Review round. The placement test renders the component directly, so nothing watched the mount itself: the page could stop rendering it, or start rendering it inside a variant behind that variant's early returns, and every test would stay green. Guarded in the source alongside the heading rule, since AboutPage sits behind the router, the config store and a query provider. Both ways of breaking it fail the guard. The signup's two handlers carry explicit return types now.
Closes #1551. Closes #1546.
The About page mount
NewsletterSignuponly ever lived inblog-sidebar.tsx, so an instance offered the digest only if its template rendered a rail. Four of the nine do not:GallerySidebar, which never mounted the formReaders of those blogs had no way to subscribe at all, while the owner saw the feature switched on in the editor.
Mounted in
AboutPageitself rather than insideBlogAboutorCommunityAbout, because both return early while their account or community query is loading or has failed and the signup depends on neither. As a side effect the community rail's form, which today sits only in the success branch, is no longer the only copy.One form per page: the rail stands down on
/about. Route based rather than theme based on purpose. Whether the rail is rendered at all is each template's own decision, and gallery shows that "declares the sidebar options unsupported" and "renders no sidebar" are not the same question, so a theme based rule would have to track nine structures while this one holds for all of them. The predicate is pure and tested (sidebarShowsNewsletter), following the app's existing "pure half, separated so it is testable" convention.The
placementprop changes only the frame. Same rules, same request, same states. On the page it is anh2rather than the rail'sh3, since the About page opens with anh1for the account or community and the rail has nothing above it, and the form keeps amax-w-mdmeasure instead of stretching across 768px of prose.#1546
The confirmation was a dead end. A typo gets the same 2xx as a real address, because double opt-in means the service cannot tell them apart, so a reader who typed
reader@typo.cmsaw "Almost there: confirm from the email we just sent" and waited for mail that would never arrive. After success the section contained no buttons, no inputs and no links: the only way out was reloading the page. There is a "use a different address" control now, which clears the field and returns to idle.Focus was dropped. The focused submit button is unmounted on success, so
document.activeElementbecamedocument.bodyand the next Tab restarted at the top of the document. Focus now follows to the new control, and back to the email field on reset.Focus is moved only when it was ours to move: when the active element is
body(what the browser leaves behind when it removes the focused node) or still inside this form. A reader who clicked elsewhere while the request was in flight keeps their place.preventScrollso a reader who scrolled away is not yanked back by a request they may have forgotten.Verification
rsbuild buildclean and the node-globals guard passes.Summary by CodeRabbit
New Features
Tests