Skip to content

Commit 551583b

Browse files
tomalaforgeclaude
andcommitted
fix(website): address review comments on the docs rework
- expire analytics cookies on every parent domain, including AdSense ones - pass the request timeout to the direct GitHub fetch calls - reset the PR reaction state when the router reuses the diff component - tell challenge authors to copy, not move, the generated Markdown Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent bde4173 commit 551583b

4 files changed

Lines changed: 73 additions & 31 deletions

File tree

website/src/app/consent.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,24 @@ export class Consent {
122122
}
123123

124124
private clearAnalyticsCookies(): void {
125-
const domain = location.hostname;
125+
const labels = location.hostname.split('.');
126+
// "www.example.com" -> ["www.example.com", "example.com"]: the tag writes on
127+
// the registrable domain, which is not always the exact host.
128+
const domains = labels
129+
.map((_, index) => labels.slice(index).join('.'))
130+
.filter((domain) => domain.includes('.'));
131+
// Analytics (_ga, _gid) plus the AdSense cookies a previous "accept" allowed.
132+
const scopes = [
133+
'',
134+
...domains.flatMap((domain) => [`; domain=${domain}`, `; domain=.${domain}`]),
135+
];
136+
126137
for (const cookie of this.document.cookie.split(';')) {
127138
const name = cookie.split('=')[0].trim();
128-
if (!name.startsWith('_ga') && !name.startsWith('_gid')) {
139+
if (!/^(_ga|_gid|_gac|_gcl|__gads)/.test(name)) {
129140
continue;
130141
}
131-
// The tag may have written the cookie on either the exact host or the
132-
// registrable domain, so expire both.
133-
for (const scope of ['', `; domain=${domain}`, `; domain=.${domain}`]) {
142+
for (const scope of scopes) {
134143
this.document.cookie = `${name}=; Path=/; Max-Age=0${scope}`;
135144
}
136145
}

website/src/app/pages/solutions/solution-diff.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
effect,
66
inject,
77
input,
8+
linkedSignal,
89
signal,
910
untracked,
1011
} from '@angular/core';
@@ -36,12 +37,20 @@ export class SolutionDiff {
3637
protected readonly theme = inject(Theme);
3738
protected readonly isDark = computed(() => this.theme.current() === 'dark');
3839

39-
protected readonly reaction = signal<'idle' | 'saving' | 'done' | 'error'>('idle');
40-
4140
/** Provided by the route resolver / router input binding. */
4241
readonly doc = input.required<Doc>();
4342
readonly pr = input.required<string>();
4443

44+
/**
45+
* The component is reused when only `:pr` changes, so the reaction state is
46+
* derived from `pr()` — it falls back to 'idle' for every new PR instead of
47+
* keeping the previous one's 'done' (which would leave the button disabled).
48+
*/
49+
protected readonly reaction = linkedSignal<string, 'idle' | 'saving' | 'done' | 'error'>({
50+
source: this.pr,
51+
computation: () => 'idle',
52+
});
53+
4554
/** 'split' on desktop, toggleable; unified is friendlier on mobile. */
4655
protected readonly mode = signal<'split' | 'unified'>(
4756
this.isBrowser && window.matchMedia('(max-width: 639px)').matches ? 'unified' : 'split',
@@ -125,10 +134,18 @@ export class SolutionDiff {
125134
location.href = this.auth.signInUrl();
126135
return;
127136
}
137+
const pr = this.pr();
128138
this.reaction.set('saving');
129-
this.http.post(`/api/pulls/${this.pr()}/react`, {}).subscribe({
130-
next: () => this.reaction.set('done'),
131-
error: () => this.reaction.set('error'),
139+
this.http.post(`/api/pulls/${pr}/react`, {}).subscribe({
140+
// A response that lands after navigating to another PR must not touch its state.
141+
next: () => this.settle(pr, 'done'),
142+
error: () => this.settle(pr, 'error'),
132143
});
133144
}
145+
146+
private settle(pr: string, state: 'done' | 'error'): void {
147+
if (this.pr() === pr) {
148+
this.reaction.set(state);
149+
}
150+
}
134151
}

website/src/content/guides/create-challenge.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,11 @@ Alternatively, you may utilize your IDE's [Nx Console extension](https://nx.dev/
5252
- A Markdown file with minimal setup will be created inside `docs/src/content/docs/challenges/${category}`.
5353

5454
:::caution
55-
The generator still writes to the legacy `docs/` folder. This website reads its challenges from
56-
`website/src/content/challenges/${category}/`, so move (or copy) the generated Markdown file there —
57-
otherwise `website/tools/generate-content.mjs` will not pick it up and your challenge will not appear
58-
on the site. The author file follows the same rule: `website/src/content/authors/${author}.json`.
55+
The generator still writes to the legacy `docs/` folder, which is still built today — so **copy**,
56+
don't move, the generated Markdown file to `website/src/content/challenges/${category}/` and keep
57+
both copies in sync until the legacy site is retired. Without that copy,
58+
`website/tools/generate-content.mjs` will not pick it up and your challenge will not appear on this
59+
site. The author file follows the same rule: `website/src/content/authors/${author}.json`.
5960
:::
6061

6162
## Challenge Creation

website/src/server/github-api.ts

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,20 @@ githubApi.get('/me', async (req, res) => {
225225
res.status(401).json({ error: 'not signed in' });
226226
return;
227227
}
228-
const response = await fetch('https://api.github.com/user', {
229-
headers: {
230-
Accept: 'application/vnd.github+json',
231-
Authorization: `Bearer ${token}`,
232-
'User-Agent': 'angular-challenges-website',
233-
},
234-
});
228+
let response: Response;
229+
try {
230+
response = await fetch(`${GITHUB_API}/user`, {
231+
headers: {
232+
Accept: 'application/vnd.github+json',
233+
Authorization: `Bearer ${token}`,
234+
'User-Agent': 'angular-challenges-website',
235+
},
236+
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
237+
});
238+
} catch {
239+
res.status(503).json({ error: 'github request failed' });
240+
return;
241+
}
235242
if (!response.ok) {
236243
res.status(401).json({ error: 'invalid token' });
237244
return;
@@ -253,16 +260,23 @@ githubApi.post('/pulls/:number/react', async (req, res) => {
253260
res.status(400).json({ error: 'invalid PR number' });
254261
return;
255262
}
256-
const response = await fetch(`https://api.github.com/repos/${REPO}/issues/${number}/reactions`, {
257-
method: 'POST',
258-
headers: {
259-
Accept: 'application/vnd.github+json',
260-
Authorization: `Bearer ${token}`,
261-
'User-Agent': 'angular-challenges-website',
262-
'Content-Type': 'application/json',
263-
},
264-
body: JSON.stringify({ content: '+1' }),
265-
});
263+
let response: Response;
264+
try {
265+
response = await fetch(`${GITHUB_API}/repos/${REPO}/issues/${number}/reactions`, {
266+
method: 'POST',
267+
headers: {
268+
Accept: 'application/vnd.github+json',
269+
Authorization: `Bearer ${token}`,
270+
'User-Agent': 'angular-challenges-website',
271+
'Content-Type': 'application/json',
272+
},
273+
body: JSON.stringify({ content: '+1' }),
274+
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
275+
});
276+
} catch {
277+
res.status(503).json({ error: 'github request failed' });
278+
return;
279+
}
266280
if (!response.ok) {
267281
res.status(response.status).json({ error: 'reaction failed' });
268282
return;
@@ -408,6 +422,7 @@ githubApi.get('/sponsors', async (_req, res) => {
408422
'User-Agent': 'angular-challenges-website',
409423
},
410424
body: JSON.stringify({ query }),
425+
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
411426
});
412427
const data = (await response.json()) as any;
413428
if (data?.errors) {

0 commit comments

Comments
 (0)