Skip to content

Commit 0911423

Browse files
Merge pull request #233 from CodeForPhilly/fix/chat-links
fix(web): make Slack links leave the SPA for the /chat redirect
2 parents 9d79a18 + 75abde3 commit 0911423

8 files changed

Lines changed: 89 additions & 8 deletions

File tree

‎apps/web/src/components/AppFooter.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ export function AppFooter() {
177177
<h2 className="text-sm font-semibold text-foreground mb-3">Connect</h2>
178178
<ul className="space-y-2 text-sm text-muted-foreground mb-4">
179179
<li>
180-
<Link to="/chat" className="hover:text-foreground transition-colors">
180+
<a href="/chat" className="hover:text-foreground transition-colors">
181181
Slack
182-
</Link>
182+
</a>
183183
</li>
184184
</ul>
185185
<div className="flex items-center gap-3 flex-wrap">

‎apps/web/src/components/ProjectCard.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export function ProjectCard({ project }: ProjectCardProps) {
7777
)}
7878
{project.links.chatChannel && (
7979
<Button asChild size="sm" variant="outline">
80-
<Link to={`/chat?channel=${encodeURIComponent(project.links.chatChannel)}`}>Chat</Link>
80+
<a href={`/chat?channel=${encodeURIComponent(project.links.chatChannel)}`}>Chat</a>
8181
</Button>
8282
)}
8383
{project.openHelpWantedCount > 0 && (

‎apps/web/src/screens/EventsIndex.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ function UpcomingList({ isStaff }: { isStaff: boolean }) {
7474
<div className="py-12 text-center">
7575
<p className="text-muted-foreground">
7676
Nothing on the calendar yet.{' '}
77-
<Link to="/chat" className="text-primary hover:underline">
77+
<a href="/chat" className="text-primary hover:underline">
7878
Join the Slack
79-
</Link>{' '}
79+
</a>{' '}
8080
and watch #announcements — or check back soon.
8181
</p>
8282
{isStaff && (

‎apps/web/src/screens/ProjectDetail.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,9 +485,9 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) {
485485
)}
486486
{project.links.chatChannel && (
487487
<Button asChild variant="outline">
488-
<Link to={`/chat?channel=${encodeURIComponent(project.links.chatChannel)}`}>
488+
<a href={`/chat?channel=${encodeURIComponent(project.links.chatChannel)}`}>
489489
Chat Channel
490-
</Link>
490+
</a>
491491
</Button>
492492
)}
493493
</div>

‎apps/web/src/screens/Volunteer.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export function Volunteer() {
5252
We coordinate everything in our Slack workspace.
5353
</p>
5454
<Button asChild variant="outline" size="sm">
55-
<Link to="/chat">Open Slack →</Link>
55+
<a href="/chat">Open Slack →</a>
5656
</Button>
5757
</div>
5858
<div className="rounded-lg border border-border bg-card p-5">

‎eslint.config.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,22 @@ export default tseslint.config(
2525
languageOptions: { globals: { ...globals.browser } },
2626
rules: {
2727
...reactHooks.configs.recommended.rules,
28+
// Paths the server answers before the SPA (specs/screens/chat.md →
29+
// "Linking to it from the app"): a router <Link> never reaches them and
30+
// renders the SPA's 404 page. Use a plain <a href> for these.
31+
'no-restricted-syntax': [
32+
'error',
33+
{
34+
selector:
35+
"JSXAttribute[name.name='to'] > Literal[value=/^\\/(chat|calendar)(\\/|\\?|$)|^\\/api\\//]",
36+
message: 'Server-handled path: use <a href>, not a router link (specs/screens/chat.md).',
37+
},
38+
{
39+
selector:
40+
"JSXAttribute[name.name='to'] > JSXExpressionContainer > TemplateLiteral > TemplateElement:first-child[value.raw=/^\\/(chat|calendar)(\\/|\\?|$)|^\\/api\\//]",
41+
message: 'Server-handled path: use <a href>, not a router link (specs/screens/chat.md).',
42+
},
43+
],
2844
},
2945
},
3046
{
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
status: in-progress
3+
depends: [chat-redirect]
4+
specs:
5+
- specs/screens/chat.md
6+
issues: [230]
7+
---
8+
9+
# Plan: In-app links to /chat navigate to the server
10+
11+
## Scope
12+
13+
Clicking "Slack" in the footer (and every other in-app link to `/chat`)
14+
shows the SPA's 404 page, because the links are React Router `<Link>`s and
15+
`/chat` is a server-side redirect the client router has no route for. A
16+
new tab or a refresh works, which is why it went unnoticed.
17+
18+
**In:** the five `/chat` links (footer, Volunteer, events empty state,
19+
project "Chat Channel" button, project card "Chat") become plain anchors;
20+
a lint rule rejects router links whose literal target starts with `/chat`,
21+
`/api/` or `/calendar`.
22+
23+
**Out:** the visible Slack entry point in the header (#231).
24+
25+
## Implements
26+
27+
- [screens/chat.md](../specs/screens/chat.md#linking-to-it-from-the-app)
28+
29+
## Approach
30+
31+
1. `<Link to="/chat…">` → `<a href="/chat…">`, keeping classes and the
32+
`Button asChild` wrappers.
33+
2. `no-restricted-syntax` in `eslint.config.js` for `apps/web`: a
34+
`to` attribute whose string literal, or template literal's first
35+
chunk, starts with one of the server prefixes.
36+
37+
## Validation
38+
39+
- [ ] `npm run type-check && npm run lint && npm test` clean.
40+
- [ ] The rule flags the five links before the fix (checked by running
41+
lint against the unfixed tree).
42+
- [ ] In the built app, clicking footer "Slack" leaves the SPA for the
43+
Slack SSO start URL instead of rendering the 404 page.
44+
45+
## Risks / unknowns
46+
47+
- The lint rule only sees literal targets; a computed `to={someVar}`
48+
slips past. Acceptable: every current server link is a literal.
49+
50+
## Notes
51+
52+
## Follow-ups

‎specs/screens/chat.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,19 @@ Use **302** (temporary) rather than 301 so we can change the destination later w
3535
- Project pages use `/chat?channel=<chatChannel>` for the "Chat Channel" button so the link looks like part of the site rather than an external Slack URL.
3636
- If we move off Slack later, every link gets re-pointed by changing this one redirect rather than chasing references through the codebase.
3737

38+
## Linking to it from the app
39+
40+
Every in-app link to `/chat` (the footer's "Slack", the Volunteer page's
41+
"Open Slack", the events empty state, the project "Chat Channel" button and
42+
the project card's "Chat") is a **full-page navigation** — a plain anchor,
43+
never a client-side router link. `/chat` exists only on the server; a
44+
router link never reaches it and lands on the SPA's 404 page instead. The
45+
same holds for every other path the server answers before the SPA does:
46+
`/api/*` and the legacy redirects (`/calendar`, laddr URL forms — see
47+
[legacy-id-mapping.md](../behaviors/legacy-id-mapping.md)). Lint rejects a
48+
router link whose literal target starts with `/chat`, `/api/` or
49+
`/calendar`.
50+
3851
## Open redirect protection
3952

4053
`channel` only feeds the `redir` value's path segment after `/messages/`; the host is hard-coded. No user input touches the host.

0 commit comments

Comments
 (0)