Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/self-hosted/src/core/i18n-strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ export type TranslationKey =
| 'newsletterCadence'
| 'newsletterSubscribe'
| 'newsletterCheckInbox'
| 'newsletterUseAnotherAddress'
| 'newsletterError'
| 'panel_configuration_instance_configuration_features_newsletter_label'
| 'panel_configuration_instance_configuration_features_newsletter_enabled_label'
Expand Down Expand Up @@ -348,6 +349,7 @@ export const translations: { en: Translations } & Record<
newsletterCadence: 'How often',
newsletterSubscribe: 'Subscribe',
newsletterCheckInbox: 'Almost there: confirm from the email we just sent.',
newsletterUseAnotherAddress: 'Use a different address',
newsletterError: 'Could not subscribe right now. Please try again.',
loading: "Loading...",
hivesigner_login_failed: 'Sign in could not be completed. Please try again.',
Expand Down
13 changes: 12 additions & 1 deletion apps/self-hosted/src/features/blog/components/about-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
useInstanceConfig,
} from '../hooks/use-instance-config';
import { safeWebsiteUrl } from '../utils/safe-website';
import { NewsletterSignup } from './newsletter-signup';

/**
* The About surface, generated from what already exists on chain: a blog
Expand All @@ -28,7 +29,17 @@ import { safeWebsiteUrl } from '../utils/safe-website';
export function AboutPage() {
useDocumentMeta({ title: t('about_title') });
const { isCommunityMode } = useInstanceConfig();
return isCommunityMode ? <CommunityAbout /> : <BlogAbout />;
return (
<>
{isCommunityMode ? <CommunityAbout /> : <BlogAbout />}
{/* Outside the variant on purpose (vision-web#1551): both of them return
early while their account or community query is loading or has failed,
and the signup depends on neither. This is the one surface every
template has, so it is where the four sidebar-less templates offer the
digest at all. */}
<NewsletterSignup placement="page" />
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
</>
);
}

function BlogAbout() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ const submitButton = () =>
// Two regions, because a failure has to interrupt: role=status is the polite
// one (the confirmation), role=alert the assertive one (the failure).
const politeRegion = () => container.querySelector('[role="status"]');
const resetControl = () =>
container.querySelector<HTMLButtonElement>('button[type="button"]');
const errorRegion = () => container.querySelector('[role="alert"]');

describe('NewsletterSignup', () => {
Expand Down Expand Up @@ -395,6 +397,92 @@ describe('NewsletterSignup', () => {
);
});

it('offers a way back from the confirmation, and puts focus on it (#1546)', async () => {
const fetchMock = vi.fn(async (_url: string, _init: RequestInit) => ({
ok: true,
}));
vi.stubGlobal('fetch', fetchMock);

await render();
// A plausible fat-finger: .cm for .com. The relay answers 2xx either way,
// because double opt-in means it cannot tell a typo from a real address,
// so the reader is told to check an inbox that will never receive anything.
await typeInto(emailInput(), 'reader@typo.cm');
// Where focus sits when a reader submits with the keyboard, on the very
// element the success state is about to remove.
submitButton()?.focus();
await submitForm();

const back = resetControl();
expect(back?.textContent).toBe('Use a different address');
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
// Without moving it, focus falls to <body> when the button is unmounted
// and the reader's next Tab restarts at the top of the document.
expect(document.activeElement).toBe(back);

await act(async () => {
back?.click();
});

// Back to a usable form, empty, focused, and no longer claiming success.
expect(form()).not.toBeNull();
expect(emailInput()?.value).toBe('');
expect(document.activeElement).toBe(emailInput());
expect(politeRegion()?.textContent).toBe('');
});

it('does not steal focus from a reader who moved on mid-request', async () => {
const pending = deferred<{ ok: boolean }>();
const fetchMock = vi.fn(
(_url: string, _init: RequestInit) => pending.promise,
);
vi.stubGlobal('fetch', fetchMock);

await render();
await typeInto(emailInput(), 'reader@example.com');
submitButton()?.focus();
await submitForm();

// They click something else on the page while the request is open.
const elsewhere = document.createElement('button');
document.body.appendChild(elsewhere);
elsewhere.focus();

await act(async () => {
pending.resolve({ ok: true });
});

expect(resetControl()).not.toBeNull();
expect(document.activeElement).toBe(elsewhere);
elsewhere.remove();
});

it('renders the same form in a page frame for the About page (#1551)', async () => {
const section = () =>
container.querySelector('[data-testid="newsletter-signup"]');

await render(<NewsletterSignup placement="page" />);
expect(section()?.className).toContain('max-w-3xl');
expect(section()?.className).not.toContain('sidebar-newsletter-section');
// h2, not h3: the About page opens with an h1 for the account or the
// community, and the rail's h3 would skip a level under it.
expect(section()?.querySelector('h2')?.textContent).toBe(
'Get new posts by email',
);
expect(section()?.querySelector('h3')).toBeNull();
// Only the frame differs: same controls, same accessible names.
expect(emailInput()?.getAttribute('aria-label')).toBe('Your email');
expect(cadenceSelect()?.getAttribute('aria-label')).toBe('How often');
expect(submitButton()?.textContent).toBe('Subscribe');

// And the default is still the rail section it was born as, where nothing
// outranks the heading so h3 is right.
await render();
expect(section()?.className).toContain('sidebar-newsletter-section');
expect(section()?.className).not.toContain('max-w-3xl');
expect(section()?.querySelector('h3')).not.toBeNull();
expect(section()?.querySelector('h2')).toBeNull();
});

it('survives an unmount mid-flight, and does not abort the request', async () => {
const pending = deferred<{ ok: boolean }>();
const fetchMock = vi.fn(
Expand Down
113 changes: 106 additions & 7 deletions apps/self-hosted/src/features/blog/components/newsletter-signup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,53 @@ import { newsletterSignupTarget, newsletterSubscribeBody } from '../utils/newsle
* the form always says "check your inbox" on success and can learn nothing
* about an address it does not own.
*/
export function NewsletterSignup(): ReactElement | null {

/**
* Where the form is being rendered. Only the frame differs: `sidebar` is the
* rail section it was born as, `page` is the wider block the About page shows
* (vision-web#1551), which is the only surface every template has. The rules,
* the request and the states are identical in both.
*/
export type NewsletterPlacement = 'sidebar' | 'page';

interface Frame {
root: string;
/**
* The heading ELEMENT, not just its size. In the rail nothing outranks it,
* so h3 is fine; the About page opens with an h1 for the account or
* community, and jumping straight to h3 would skip a level on the one page
* here that has a real heading outline.
*/
heading: 'h2' | 'h3';
title: string;
blurb: string;
/**
* The rail is narrow enough to be its own measure. The About column is 3xl
* prose, where a full-width email field looks like a mistake.
*/
form: string;
}

const FRAME: Record<NewsletterPlacement, Frame> = {
sidebar: {
root: 'border-t border-theme pt-4 mt-4 sidebar-newsletter-section',
heading: 'h3',
title: 'text-sm font-semibold mb-1',
blurb: 'text-xs text-theme-muted mb-2',
form: 'flex flex-col gap-2',
},
page: {
root: 'max-w-3xl mx-auto border-t border-theme pt-6 mt-10 page-newsletter-section',
heading: 'h2',
title: 'heading-theme text-xl mb-1',
blurb: 'text-theme-secondary leading-relaxed mb-4',
form: 'flex flex-col gap-2 max-w-md',
},
};

export function NewsletterSignup({
placement = 'sidebar',
}: { placement?: NewsletterPlacement } = {}): ReactElement | null {
const target = InstanceConfigManager.useConfig(({ configuration }) =>
newsletterSignupTarget({
username: configuration.instanceConfiguration.username,
Expand All @@ -38,8 +84,36 @@ export function NewsletterSignup(): ReactElement | null {
};
}, []);

const root = useRef<HTMLDivElement>(null);
const resetButton = useRef<HTMLButtonElement>(null);
const emailField = useRef<HTMLInputElement>(null);
/**
* Set only for the two transitions that REMOVE whatever holds focus: the
* form being replaced by the confirmation, and the confirmation being
* replaced by the form again. Never on first render, or the page would hand
* focus to a sidebar form the moment it loads.
*/
const restoreFocus = useRef(false);
useEffect(() => {
if (!restoreFocus.current) return;
restoreFocus.current = false;
const next = state === 'done' ? resetButton.current : state === 'idle' ? emailField.current : null;
if (!next) return;
// The browser drops focus to <body> when the focused element is removed,
// so that (or focus still inside this form) means the reader was here and
// has nowhere to stand. Anything else means they moved on while the
// request was in flight, and their place is theirs to keep.
const active = document.activeElement;
if (active && active !== document.body && !root.current?.contains(active)) return;
// Without preventScroll, a reader who has scrolled away from the form is
// yanked back to it by a request they may have forgotten about.
next.focus({ preventScroll: true });
}, [state]);

if (!target) return null;
const isCommunity = target.type === 'community';
const frame = FRAME[placement];
const Heading = frame.heading;

const submit = async (e: FormEvent) => {
e.preventDefault();
Expand All @@ -51,16 +125,31 @@ export function NewsletterSignup(): ReactElement | null {
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(newsletterSubscribeBody(target, email, cadence)),
});
if (mounted.current) setState(res.ok ? 'done' : 'error');
if (!mounted.current) return;
// Only success swaps the form out, so only success has focus to rescue.
restoreFocus.current = res.ok;
setState(res.ok ? 'done' : 'error');
} catch {
if (mounted.current) setState('error');
}
};

/**
* The way back from the confirmation (vision-web#1546). A typo gets the same
* 2xx as a real address, because double opt-in means the service cannot tell
* one from the other, so without this the reader waits for mail that will
* never arrive and the only way out is reloading the page.
*/
const useAnotherAddress = () => {
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated
restoreFocus.current = true;
setEmail('');
setState('idle');
};

return (
<div className="border-t border-theme pt-4 mt-4 sidebar-newsletter-section" data-testid="newsletter-signup">
<h3 className="text-sm font-semibold mb-1">{t('newsletterTitle')}</h3>
<p className="text-xs text-theme-muted mb-2">{t(isCommunity ? 'newsletterCommunityBlurb' : 'newsletterBlurb')}</p>
<div ref={root} className={frame.root} data-testid="newsletter-signup">
<Heading className={frame.title}>{t('newsletterTitle')}</Heading>
<p className={frame.blurb}>{t(isCommunity ? 'newsletterCommunityBlurb' : 'newsletterBlurb')}</p>
{/* Both regions are mounted from the first render, per the live-region
contract: a region that appears at the same moment as its message is
often not announced. Two of them, as community-join-button.tsx does,
Expand All @@ -73,12 +162,22 @@ export function NewsletterSignup(): ReactElement | null {
message={state === 'error' ? t('newsletterError') : null}
className="text-xs block mb-1 text-red-500 dark:text-red-400"
/>
{state !== 'done' && (
<form onSubmit={submit} className="flex flex-col gap-2">
{state === 'done' ? (
<button
type="button"
ref={resetButton}
onClick={useAnotherAddress}
className="text-xs underline text-theme-muted"
>
{t('newsletterUseAnotherAddress')}
</button>
) : (
<form onSubmit={submit} className={frame.form}>
<input
type="email"
required
autoComplete="email"
ref={emailField}
value={email}
onChange={(e) => setEmail(e.target.value)}
placeholder={t('newsletterEmail')}
Expand Down
20 changes: 18 additions & 2 deletions apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { formatMonthYear, InstanceConfigManager, t } from "@/core";
import { useRouterState } from "@tanstack/react-router";
import { NewsletterSignup } from "../components/newsletter-signup";
import { sidebarShowsNewsletter } from "../utils/newsletter-signup-target";
import { useAuth } from "@/features/auth";
import { InlineError } from "@/features/shared/inline-error";
import {
Expand Down Expand Up @@ -39,7 +41,20 @@ export function BlogSidebar() {
return <BlogSidebarContent username={username} />;
}

/**
* One signup form per page. The About page carries it in its content column
* (vision-web#1551), which is the only surface every template has, so the rail
* stands down there rather than showing a second copy. Decided by route rather
* than by theme because whether this rail is rendered at all is each
* template's own choice, and four of the nine do not render it.
*/
function useShowSidebarNewsletter(): boolean {
const pathname = useRouterState({ select: (state) => state.location.pathname });
return sidebarShowsNewsletter(pathname);
}

function BlogSidebarContent({ username }: { username: string }) {
const showNewsletter = useShowSidebarNewsletter();
const { data } = useQuery({
...getAccountFullQueryOptions(username),
enabled: !!username,
Expand Down Expand Up @@ -104,7 +119,7 @@ function BlogSidebarContent({ username }: { username: string }) {
</div>
</div>
)}
<NewsletterSignup />
{showNewsletter && <NewsletterSignup />}
{data && (
<div className="border-t border-theme pt-4 mt-4 sidebar-hive-info-section">
<div className="text-xs font-medium mb-2 text-theme-muted">
Expand Down Expand Up @@ -171,6 +186,7 @@ function CommunitySidebarShell({ children }: { children: ReactNode }) {
}

function CommunitySidebar() {
const showNewsletter = useShowSidebarNewsletter();
const {
data: community,
isEnabled,
Expand Down Expand Up @@ -319,7 +335,7 @@ function CommunitySidebar() {
<CommunityJoinButton communityId={communityId} />
</div>

<NewsletterSignup />
{showNewsletter && <NewsletterSignup />}
<div className="border-t border-theme pt-4 mt-4 sidebar-hive-info-section">
<div className="text-xs font-medium mb-2 text-theme-muted">
{t("community_info")}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, it } from 'vitest';
import { newsletterSignupTarget, newsletterSubscribeBody } from './newsletter-signup-target';
import {
newsletterSignupTarget,
newsletterSubscribeBody,
sidebarShowsNewsletter,
} from './newsletter-signup-target';

describe('newsletterSignupTarget', () => {
const managedBlog = { username: 'Alice', managed: true, siteTitle: 'Alice Writes' };
Expand Down Expand Up @@ -43,3 +47,23 @@ describe('newsletterSignupTarget', () => {
});
});
});

describe('sidebarShowsNewsletter', () => {
it('stands down on the About page, which carries its own', () => {
// One form per page: the About page renders it in the content column, and
// it is the only surface every template has.
expect(sidebarShowsNewsletter('/about')).toBe(false);
expect(sidebarShowsNewsletter('/about/')).toBe(false);
expect(sidebarShowsNewsletter('/about//')).toBe(false);
});

it('shows it everywhere else, including paths that merely start with it', () => {
expect(sidebarShowsNewsletter('/')).toBe(true);
expect(sidebarShowsNewsletter('/@alice/hello')).toBe(true);
expect(sidebarShowsNewsletter('/search')).toBe(true);
// Not a prefix match: a post that happens to live under /about-something
// is an ordinary page and still gets the rail's form.
expect(sidebarShowsNewsletter('/aboutus')).toBe(true);
expect(sidebarShowsNewsletter('/about/extra')).toBe(true);
});
});
Loading
Loading