Skip to content

Newsletter: signup on the About page, and a way back from the confirmation - #1552

Merged
feruzm merged 3 commits into
developfrom
feature/newsletter-about-embed
Aug 20, 2026
Merged

Newsletter: signup on the About page, and a way back from the confirmation#1552
feruzm merged 3 commits into
developfrom
feature/newsletter-about-embed

Conversation

@feruzm

@feruzm feruzm commented Aug 20, 2026

Copy link
Copy Markdown
Member

Closes #1551. Closes #1546.

The About page mount

NewsletterSignup only ever lived in blog-sidebar.tsx, so an instance offered the digest only if its template rendered a rail. Four of the nine do not:

template rail form before this
medium, minimal, magazine, developer, modern-gradient DefaultShell renders the seam yes
journal, reader, terminal their own Shell drops the seam no
gallery replaces the seam with GallerySidebar, which never mounted the form no

Readers of those blogs had no way to subscribe at all, while the owner saw the feature switched on in the editor.

Mounted in AboutPage itself rather than inside BlogAbout or CommunityAbout, 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 placement prop changes only the frame. Same rules, same request, same states. On the page it is an h2 rather than the rail's h3, since the About page opens with an h1 for the account or community and the rail has nothing above it, and the form keeps a max-w-md measure 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.cm saw "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.activeElement became document.body and 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. preventScroll so a reader who scrolled away is not yanked back by a request they may have forgotten.

Verification

Summary by CodeRabbit

  • New Features

    • Added newsletter signup to the About page.
    • Added a full-page newsletter layout alongside the sidebar version.
    • Added a “Use a different address” option after successful signup.
    • Improved focus handling when submitting, confirming, or resetting the form.
    • Newsletter visibility now adapts by page, remaining hidden from the About page sidebar.
  • Tests

    • Added coverage for signup placement, focus behavior, confirmation recovery, and route-based visibility.

…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.
@qodo-code-review

qodo-code-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Add About-page newsletter signup and recoverable confirmation

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Adds newsletter signup to About pages, covering templates without standard sidebars.
• Suppresses sidebar duplicates on About routes while preserving signup elsewhere.
• Adds recoverable confirmation and focus-safe address correction after successful submissions.
Diagram

graph TD
R["Current route"] --> P{"About page?"}
P -- Yes --> A["About content"] --> N["Newsletter signup"] --> API["Subscribe API"] --> C["Confirmation state"] --> B["Change address"]
P -- No --> S["Blog sidebar"] --> N
B --> N
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Theme-aware sidebar rules
  • ➕ Could preserve sidebar placement for each template individually.
  • ➕ Keeps newsletter rendering decisions alongside theme structure.
  • ➖ Requires tracking nine template layouts and future structural changes.
  • ➖ Does not cleanly handle gallery’s custom sidebar behavior.
  • ➖ Couples a route-level uniqueness rule to presentation themes.
2. Embed signup in About variants
  • ➕ Places each signup directly beside its blog or community content.
  • ➕ Avoids a wrapper-level sibling component.
  • ➖ Duplicates integration across blog and community variants.
  • ➖ Hides the signup during independent account or community loading failures.
  • ➖ Makes future variant additions responsible for repeating the integration.

Recommendation: Keep the PR’s route-based placement and shared component. Mounting the signup at AboutPage guarantees coverage across templates and query states, while the pure pathname predicate enforces one form per page without coupling behavior to theme internals.

Files changed (7) +265 / -11

Enhancement (4) +46 / -3
i18n-strings.tsAdd newsletter address-reset translation +2/-0

Add newsletter address-reset translation

• Adds the typed English translation used by the confirmation-state recovery control.

apps/self-hosted/src/core/i18n-strings.ts

about-page.tsxMount newsletter signup on every About page +12/-1

Mount newsletter signup on every About page

• Renders the page-framed newsletter signup outside blog and community query-dependent variants. This keeps signup available across all templates and during About content loading or failure states.

apps/self-hosted/src/features/blog/components/about-page.tsx

blog-sidebar.tsxSuppress sidebar signup on About routes +18/-2

Suppress sidebar signup on About routes

• Reads the active pathname for both blog and community sidebars and hides their newsletter form on About. This prevents duplicate forms now that About renders its own signup.

apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx

newsletter-signup-target.tsAdd pure About-route visibility predicate +14/-0

Add pure About-route visibility predicate

• Adds a testable pathname predicate that disables sidebar newsletter rendering only for the canonical About route after trailing-slash normalization.

apps/self-hosted/src/features/blog/utils/newsletter-signup-target.ts

Tests (2) +113 / -1
newsletter-signup.test.tsxTest page framing and recoverable confirmation behavior +88/-0

Test page framing and recoverable confirmation behavior

• Covers address reset, focus transfer after success, focus restoration to the email field, and protection against stealing focus during pending requests. Also verifies semantic headings and styling for page and sidebar placements.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx

newsletter-signup-target.test.tsTest route-based sidebar newsletter visibility +25/-1

Test route-based sidebar newsletter visibility

• Verifies exact About paths, including trailing slashes, hide the sidebar form while similarly prefixed and unrelated routes continue showing it.

apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts

Other (1) +106 / -7
newsletter-signup.tsxSupport page placement and focus-safe confirmation recovery +106/-7

Support page placement and focus-safe confirmation recovery

• Introduces shared sidebar and page frames with appropriate heading levels and form widths. Adds a confirmation recovery control that clears the address, restores the form, and moves focus only when the signup still owns it.

apps/self-hosted/src/features/blog/components/newsletter-signup.tsx

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (6) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (5)

Grey Divider


Action required

1. Tests assert translated English 📜 Skill insight ≡ Correctness
Description
The new tests assert English translations such as Use a different address instead of the
corresponding i18n key. This makes the tests locale-dependent and violates the required key-based
assertion convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R416-417]

+    const back = resetControl();
+    expect(back?.textContent).toBe('Use a different address');
Relevance

●●● Strong

Recent history accepts localization of hardcoded English strings, matching this locale-dependent
assertion.

PR-#690

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test directly compares the reset control text to the English translation `Use a different
address`; additional added assertions similarly compare headings, labels, and controls to English
output.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]
Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace assertions against translated English newsletter text with assertions against i18n key strings under the test environment's translation mock.
## Issue Context
Tests must assert i18n keys rather than locale-specific translated output. Apply the same correction to the other newly added English text assertions in this test.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Tests assert translated English 📜 Skill insight ≡ Correctness
Description
The new tests assert English translations such as Use a different address instead of the
corresponding i18n key. This makes the tests locale-dependent and violates the required key-based
assertion convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R416-417]

+    const back = resetControl();
+    expect(back?.textContent).toBe('Use a different address');
Relevance

●●● Strong

Recent history accepts localization of hardcoded English strings, matching this locale-dependent
assertion.

PR-#690

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test directly compares the reset control text to the English translation `Use a different
address`; additional added assertions similarly compare headings, labels, and controls to English
output.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]
Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace assertions against translated English newsletter text with assertions against i18n key strings under the test environment's translation mock.
## Issue Context
Tests must assert i18n keys rather than locale-specific translated output. Apply the same correction to the other newly added English text assertions in this test.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Tests assert translated English ✗ Dismissed 📜 Skill insight ≡ Correctness
Description
The new tests assert English translations such as Use a different address instead of the
corresponding i18n key. This makes the tests locale-dependent and violates the required key-based
assertion convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R416-417]

+    const back = resetControl();
+    expect(back?.textContent).toBe('Use a different address');
Relevance

●●● Strong

Recent history accepts localization of hardcoded English strings, matching this locale-dependent
assertion.

PR-#690

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test directly compares the reset control text to the English translation `Use a different
address`; additional added assertions similarly compare headings, labels, and controls to English
output.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]
Skill: add-test: Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace assertions against translated English newsletter text with assertions against i18n key strings under the test environment's translation mock.
## Issue Context
Tests must assert i18n keys rather than locale-specific translated output. Apply the same correction to the other newly added English text assertions in this test.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. useAnotherAddress lacks return type ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The newly introduced useAnotherAddress function has no explicit return type annotation. This
violates the requirement for explicit types in new TypeScript functions.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143]

+  const useAnotherAddress = () => {
Relevance

●●● Strong

Recent same-file feedback accepted explicit return-type annotations, and this is a deterministic
TypeScript rule fix.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added declaration const useAnotherAddress = () => relies on inferred return typing rather than
explicitly declaring void, contrary to the TypeScript rule.

Rule 2668119: Disallow implicit and any types in new TypeScript code
apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an explicit `: void` return type to the new `useAnotherAddress` function.
## Issue Context
The function only updates refs and component state and does not return a value.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. useAnotherAddress lacks return type ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The newly introduced useAnotherAddress function has no explicit return type annotation. This
violates the requirement for explicit types in new TypeScript functions.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143]

+  const useAnotherAddress = () => {
Relevance

●●● Strong

Recent same-file feedback accepted explicit return-type annotations, and this is a deterministic
TypeScript rule fix.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added declaration const useAnotherAddress = () => relies on inferred return typing rather than
explicitly declaring void, contrary to the TypeScript rule.

Rule 2668119: Disallow implicit and any types in new TypeScript code
apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an explicit `: void` return type to the new `useAnotherAddress` function.
## Issue Context
The function only updates refs and component state and does not return a value.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. useAnotherAddress lacks return type ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The newly introduced useAnotherAddress function has no explicit return type annotation. This
violates the requirement for explicit types in new TypeScript functions.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143]

+  const useAnotherAddress = () => {
Relevance

●●● Strong

Recent same-file feedback accepted explicit return-type annotations, and this is a deterministic
TypeScript rule fix.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added declaration const useAnotherAddress = () => relies on inferred return typing rather than
explicitly declaring void, contrary to the TypeScript rule.

Rule 2668119: Disallow implicit and any types in new TypeScript code
apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an explicit `: void` return type to the new `useAnotherAddress` function.
## Issue Context
The function only updates refs and component state and does not return a value.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (3)
7. AboutPage mount lacks coverage 📘 Rule violation ▣ Testability
Description
The PR adds a newsletter mount to AboutPage, but the added page-placement test renders
NewsletterSignup directly and never exercises the page integration. A regression in the actual
About route mount or its interaction with the page variants would therefore pass the new tests.
Code

apps/self-hosted/src/features/blog/components/about-page.tsx[40]

+      <NewsletterSignup placement="page" />
Relevance

●● Moderate

Coverage concerns are plausible, but history lacks a close AboutPage integration-test precedent.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
AboutPage now mounts `, while the added test calls render()` directly rather than rendering
AboutPage. Thus the new page-level integration path required by the rule is not covered.

Rule 2667972: Require tests for all new functional code paths
apps/self-hosted/src/features/blog/components/about-page.tsx[32-40]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-463]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an automated test that renders `AboutPage` and verifies that the newsletter signup appears for both blog and community variants.
## Issue Context
The existing placement test renders `NewsletterSignup` directly, so it does not execute the newly modified `AboutPage` code path.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/about-page.tsx[32-42]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-484]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. AboutPage mount lacks coverage 📘 Rule violation ▣ Testability
Description
The PR adds a newsletter mount to AboutPage, but the added page-placement test renders
NewsletterSignup directly and never exercises the page integration. A regression in the actual
About route mount or its interaction with the page variants would therefore pass the new tests.
Code

apps/self-hosted/src/features/blog/components/about-page.tsx[40]

+      <NewsletterSignup placement="page" />
Relevance

●● Moderate

Coverage concerns are plausible, but history lacks a close AboutPage integration-test precedent.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
AboutPage now mounts `, while the added test calls render()` directly rather than rendering
AboutPage. Thus the new page-level integration path required by the rule is not covered.

Rule 2667972: Require tests for all new functional code paths
apps/self-hosted/src/features/blog/components/about-page.tsx[32-40]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-463]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an automated test that renders `AboutPage` and verifies that the newsletter signup appears for both blog and community variants.
## Issue Context
The existing placement test renders `NewsletterSignup` directly, so it does not execute the newly modified `AboutPage` code path.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/about-page.tsx[32-42]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-484]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. AboutPage mount lacks coverage 📘 Rule violation ▣ Testability
Description
The PR adds a newsletter mount to AboutPage, but the added page-placement test renders
NewsletterSignup directly and never exercises the page integration. A regression in the actual
About route mount or its interaction with the page variants would therefore pass the new tests.
Code

apps/self-hosted/src/features/blog/components/about-page.tsx[40]

+      <NewsletterSignup placement="page" />
Relevance

●● Moderate

Coverage concerns are plausible, but history lacks a close AboutPage integration-test precedent.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
AboutPage now mounts `, while the added test calls render()` directly rather than rendering
AboutPage. Thus the new page-level integration path required by the rule is not covered.

Rule 2667972: Require tests for all new functional code paths
apps/self-hosted/src/features/blog/components/about-page.tsx[32-40]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-463]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an automated test that renders `AboutPage` and verifies that the newsletter signup appears for both blog and community variants.
## Issue Context
The existing placement test renders `NewsletterSignup` directly, so it does not execute the newly modified `AboutPage` code path.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/about-page.tsx[32-42]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-484]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

10. Newsletter tests use invalid paths 📜 Skill insight ⌂ Architecture
Description
New tests are added to co-located .test.tsx and .test.ts files instead of the corresponding
src/specs/ paths using .spec names. This violates both the required test directory mapping and
naming convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400]

+  it('offers a way back from the confirmation, and puts focus on it (#1546)', async () => {
Relevance

● Weak

The same test-location and naming request was explicitly rejected for this PR.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added component tests are in src/features/blog/components/newsletter-signup.test.tsx, and the
added utility tests are in src/features/blog/utils/newsletter-signup-target.test.ts; both are
co-located and use the prohibited .test naming pattern.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-400]
apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-52]
Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Move the new newsletter tests out of the feature source directories and rename them to use the required `.spec.tsx` or `.spec.ts` pattern.
## Issue Context
Feature tests must map from `src/features/<domain>/` to `src/specs/features/<domain>/` and must not remain co-located with production code.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-484]
- apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. Layout test inspects classes 📘 Rule violation ▣ Testability
Description
The new page-frame test asserts internal Tailwind and component class names rather than user-visible
behavior. These assertions couple the test to the styling implementation even though the same
behavior can be checked through semantic headings and controls.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R464-465]

+    expect(section()?.className).toContain('max-w-3xl');
+    expect(section()?.className).not.toContain('sidebar-newsletter-section');
Relevance

● Weak

Recent reviewers rejected replacing or avoiding implementation-detail test assertions in favor of
runtime behavior.

PR-#1481
PR-#1364

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test explicitly inspects whether className contains max-w-3xl and excludes
sidebar-newsletter-section, which are internal styling details rather than rendered behavior.

Rule 2667994: UI tests must verify user-visible behavior rather than internal implementation details
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-465]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remove assertions against internal `className` values and verify the page placement through rendered semantics and user-observable behavior.
## Issue Context
The existing heading-level and accessible-control assertions provide behavior-oriented coverage without coupling the test to Tailwind implementation details.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-471]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[479-483]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Newsletter tests use invalid paths 📜 Skill insight ⌂ Architecture
Description
New tests are added to co-located .test.tsx and .test.ts files instead of the corresponding
src/specs/ paths using .spec names. This violates both the required test directory mapping and
naming convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400]

+  it('offers a way back from the confirmation, and puts focus on it (#1546)', async () => {
Relevance

● Weak

The same test-location and naming request was explicitly rejected for this PR.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added component tests are in src/features/blog/components/newsletter-signup.test.tsx, and the
added utility tests are in src/features/blog/utils/newsletter-signup-target.test.ts; both are
co-located and use the prohibited .test naming pattern.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-400]
apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-52]
Skill: add-test: Skill: add-test: Skill: add-test: Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Move the new newsletter tests out of the feature source directories and rename them to use the required `.spec.tsx` or `.spec.ts` pattern.
## Issue Context
Feature tests must map from `src/features/<domain>/` to `src/specs/features/<domain>/` and must not remain co-located with production code.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-484]
- apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View low (3)
13. Layout test inspects classes 📘 Rule violation ▣ Testability
Description
The new page-frame test asserts internal Tailwind and component class names rather than user-visible
behavior. These assertions couple the test to the styling implementation even though the same
behavior can be checked through semantic headings and controls.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R464-465]

+    expect(section()?.className).toContain('max-w-3xl');
+    expect(section()?.className).not.toContain('sidebar-newsletter-section');
Relevance

● Weak

Recent reviewers rejected replacing or avoiding implementation-detail test assertions in favor of
runtime behavior.

PR-#1481
PR-#1364

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test explicitly inspects whether className contains max-w-3xl and excludes
sidebar-newsletter-section, which are internal styling details rather than rendered behavior.

Rule 2667994: UI tests must verify user-visible behavior rather than internal implementation details
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-465]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remove assertions against internal `className` values and verify the page placement through rendered semantics and user-observable behavior.
## Issue Context
The existing heading-level and accessible-control assertions provide behavior-oriented coverage without coupling the test to Tailwind implementation details.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-471]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[479-483]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


14. Newsletter tests use invalid paths 📜 Skill insight ⌂ Architecture
Description
New tests are added to co-located .test.tsx and .test.ts files instead of the corresponding
src/specs/ paths using .spec names. This violates both the required test directory mapping and
naming convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400]

+  it('offers a way back from the confirmation, and puts focus on it (#1546)', async () => {
Relevance

● Weak

The same test-location and naming request was explicitly rejected for this PR.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added component tests are in src/features/blog/components/newsletter-signup.test.tsx, and the
added utility tests are in src/features/blog/utils/newsletter-signup-target.test.ts; both are
co-located and use the prohibited .test naming pattern.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-400]
apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-52]
Skill: add-test: Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Move the new newsletter tests out of the feature source directories and rename them to use the required `.spec.tsx` or `.spec.ts` pattern.
## Issue Context
Feature tests must map from `src/features/<domain>/` to `src/specs/features/<domain>/` and must not remain co-located with production code.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-484]
- apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


15. Layout test inspects classes 📘 Rule violation ▣ Testability
Description
The new page-frame test asserts internal Tailwind and component class names rather than user-visible
behavior. These assertions couple the test to the styling implementation even though the same
behavior can be checked through semantic headings and controls.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R464-465]

+    expect(section()?.className).toContain('max-w-3xl');
+    expect(section()?.className).not.toContain('sidebar-newsletter-section');
Relevance

● Weak

Recent reviewers rejected replacing or avoiding implementation-detail test assertions in favor of
runtime behavior.

PR-#1481
PR-#1364

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test explicitly inspects whether className contains max-w-3xl and excludes
sidebar-newsletter-section, which are internal styling details rather than rendered behavior.

Rule 2667994: UI tests must verify user-visible behavior rather than internal implementation details
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-465]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remove assertions against internal `className` values and verify the page placement through rendered semantics and user-observable behavior.
## Issue Context
The existing heading-level and accessible-control assertions provide behavior-oriented coverage without coupling the test to Tailwind implementation details.
## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-471]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[479-483]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown

Greptile Summary

The 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.

  • Introduces page and sidebar presentation frames for the shared newsletter flow.
  • Adds route-aware sidebar visibility and mounts the page form independently of About-page query outcomes.
  • Refactors both About variants through a common frame so loading, failure, and success states have a page heading.
  • Adds source-structure and component behavior coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

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

Comment thread apps/self-hosted/src/features/blog/components/about-page.tsx
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@feruzm, you've reached your PR review limit, so we couldn't start this review.

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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ca8707c-8217-4a67-9423-933aafa6c882

📥 Commits

Reviewing files that changed from the base of the PR and between f6613ee and 2c517f1.

📒 Files selected for processing (3)
  • apps/self-hosted/src/features/blog/components/about-page-structure.test.ts
  • apps/self-hosted/src/features/blog/components/about-page.tsx
  • apps/self-hosted/src/features/blog/components/newsletter-signup.tsx

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9e78174a-472b-4318-a123-ace6bf9914b2

📥 Commits

Reviewing files that changed from the base of the PR and between 41e5f56 and f6613ee.

📒 Files selected for processing (7)
  • apps/self-hosted/src/core/i18n-strings.ts
  • apps/self-hosted/src/features/blog/components/about-page.tsx
  • apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx
  • apps/self-hosted/src/features/blog/components/newsletter-signup.tsx
  • apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx
  • apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts
  • apps/self-hosted/src/features/blog/utils/newsletter-signup-target.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The 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 /about to prevent duplicates.

Changes

Newsletter signup experience

Layer / File(s) Summary
Signup states, placement, and translations
apps/self-hosted/src/core/i18n-strings.ts, apps/self-hosted/src/features/blog/components/newsletter-signup.tsx, apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx
NewsletterSignup supports sidebar and page layouts. Successful submission shows a translated reset control. Focus restoration and reset behavior are covered by tests.
Route-aware sidebar visibility
apps/self-hosted/src/features/blog/utils/newsletter-signup-target.ts, apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts, apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx
The blog and community sidebars hide the signup on normalized /about paths and show it on other paths.
About page signup placement
apps/self-hosted/src/features/blog/components/about-page.tsx
AboutPage renders NewsletterSignup with placement="page" after the selected about content.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f6613

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
Loading

Possibly related PRs

Poem

A rabbit hops by the signup form,
Two layouts now greet the morn.
Focus stays where readers stand,
A fresh address is close at hand.
/about gets one form, neat and bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two primary changes: adding signup to the About page and providing recovery from confirmation.
Linked Issues check ✅ Passed The changes satisfy the linked issues by adding About-page signup, preventing duplicate sidebar forms, and restoring recovery and focus behavior [#1551, #1546].
Out of Scope Changes check ✅ Passed All changes support the linked objectives, including implementation, translations, route handling, accessibility behavior, and focused tests.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/newsletter-about-embed

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

qodo-code-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📜 Skill insights (1)

Grey Divider


Action required

1. Tests assert translated English ✗ Dismissed 📜 Skill insight ≡ Correctness
Description
The new tests assert English translations such as Use a different address instead of the
corresponding i18n key. This makes the tests locale-dependent and violates the required key-based
assertion convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R416-417]

+    const back = resetControl();
+    expect(back?.textContent).toBe('Use a different address');
Relevance

●●● Strong

Recent history accepts localization of hardcoded English strings, matching this locale-dependent
assertion.

PR-#690

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test directly compares the reset control text to the English translation `Use a different
address`; additional added assertions similarly compare headings, labels, and controls to English
output.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]
Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace assertions against translated English newsletter text with assertions against i18n key strings under the test environment's translation mock.

## Issue Context
Tests must assert i18n keys rather than locale-specific translated output. Apply the same correction to the other newly added English text assertions in this test.

## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[416-417]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[468-475]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. useAnotherAddress lacks return type ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The newly introduced useAnotherAddress function has no explicit return type annotation. This
violates the requirement for explicit types in new TypeScript functions.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143]

+  const useAnotherAddress = () => {
Relevance

●●● Strong

Recent same-file feedback accepted explicit return-type annotations, and this is a deterministic
TypeScript rule fix.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added declaration const useAnotherAddress = () => relies on inferred return typing rather than
explicitly declaring void, contrary to the TypeScript rule.

Rule 2668119: Disallow implicit and any types in new TypeScript code
apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an explicit `: void` return type to the new `useAnotherAddress` function.

## Issue Context
The function only updates refs and component state and does not return a value.

## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.tsx[143-147]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. AboutPage mount lacks coverage 📘 Rule violation ▣ Testability
Description
The PR adds a newsletter mount to AboutPage, but the added page-placement test renders
NewsletterSignup directly and never exercises the page integration. A regression in the actual
About route mount or its interaction with the page variants would therefore pass the new tests.
Code

apps/self-hosted/src/features/blog/components/about-page.tsx[40]

+      <NewsletterSignup placement="page" />
Relevance

●● Moderate

Coverage concerns are plausible, but history lacks a close AboutPage integration-test precedent.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
AboutPage now mounts <NewsletterSignup placement="page" />, while the added test calls
render(<NewsletterSignup placement="page" />) directly rather than rendering AboutPage. Thus the
new page-level integration path required by the rule is not covered.

Rule 2667972: Require tests for all new functional code paths
apps/self-hosted/src/features/blog/components/about-page.tsx[32-40]
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-463]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Add an automated test that renders `AboutPage` and verifies that the newsletter signup appears for both blog and community variants.

## Issue Context
The existing placement test renders `NewsletterSignup` directly, so it does not execute the newly modified `AboutPage` code path.

## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/about-page.tsx[32-42]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[459-484]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

4. Newsletter tests use invalid paths 📜 Skill insight ⌂ Architecture
Description
New tests are added to co-located .test.tsx and .test.ts files instead of the corresponding
src/specs/ paths using .spec names. This violates both the required test directory mapping and
naming convention.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400]

+  it('offers a way back from the confirmation, and puts focus on it (#1546)', async () => {
Relevance

● Weak

The same test-location and naming request was explicitly rejected for this PR.

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added component tests are in src/features/blog/components/newsletter-signup.test.tsx, and the
added utility tests are in src/features/blog/utils/newsletter-signup-target.test.ts; both are
co-located and use the prohibited .test naming pattern.

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-400]
apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-52]
Skill: add-test

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Move the new newsletter tests out of the feature source directories and rename them to use the required `.spec.tsx` or `.spec.ts` pattern.

## Issue Context
Feature tests must map from `src/features/<domain>/` to `src/specs/features/<domain>/` and must not remain co-located with production code.

## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[400-484]
- apps/self-hosted/src/features/blog/utils/newsletter-signup-target.test.ts[51-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Layout test inspects classes 📘 Rule violation ▣ Testability
Description
The new page-frame test asserts internal Tailwind and component class names rather than user-visible
behavior. These assertions couple the test to the styling implementation even though the same
behavior can be checked through semantic headings and controls.
Code

apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[R464-465]

+    expect(section()?.className).toContain('max-w-3xl');
+    expect(section()?.className).not.toContain('sidebar-newsletter-section');
Relevance

● Weak

Recent reviewers rejected replacing or avoiding implementation-detail test assertions in favor of
runtime behavior.

PR-#1481
PR-#1364

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test explicitly inspects whether className contains max-w-3xl and excludes
sidebar-newsletter-section, which are internal styling details rather than rendered behavior.

Rule 2667994: UI tests must verify user-visible behavior rather than internal implementation details
apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-465]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remove assertions against internal `className` values and verify the page placement through rendered semantics and user-observable behavior.

## Issue Context
The existing heading-level and accessible-control assertions provide behavior-oriented coverage without coupling the test to Tailwind implementation details.

## Fix Focus Areas
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[464-471]
- apps/self-hosted/src/features/blog/components/newsletter-signup.test.tsx[479-483]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 82 rules
✅ Skills: 6 invoked
  add-feature
  add-query
  add-sdk-mutation
  add-test
  code-review
  debug
✅ Web pages:
  +2 more
Review mode: 🧠 Deep: This changes several independent behavioral paths—route-based rendering, two sidebar variants, async success/reset focus management, and responsive framing—with enough subtle UI and accessibility interactions that a redundant review is warranted.

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread apps/self-hosted/src/features/blog/components/about-page.tsx
Comment thread apps/self-hosted/src/features/blog/components/newsletter-signup.tsx Outdated
feruzm added 2 commits August 20, 2026 08:23
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.
@feruzm
feruzm merged commit 6746575 into develop Aug 20, 2026
10 checks passed
@feruzm
feruzm deleted the feature/newsletter-about-embed branch August 20, 2026 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant