Skip to content

fix: add a proper 404 page#1355

Open
kevinw-openai wants to merge 1 commit intoopenclaw:mainfrom
kevinw-openai:codex/fix-404-page-layout
Open

fix: add a proper 404 page#1355
kevinw-openai wants to merge 1 commit intoopenclaw:mainfrom
kevinw-openai:codex/fix-404-page-layout

Conversation

@kevinw-openai
Copy link
Copy Markdown

Why

Visiting a non-existent route currently falls back to TanStack Router's default Not Found paragraph.
That leaves the page looking unstyled and broken, with no clear recovery action for the user.

What changed

  • registered an app-level 404 handler via defaultNotFoundComponent
  • added a dedicated NotFoundPage component with real layout and styling
  • kept the final UI intentionally minimal:
    • 404 • Page not found
    • main explanatory copy
    • a single Return home CTA
  • added tests to cover both the router-level registration and the rendered 404 UI

Before

chrome-capture-2026-03-28 (1)
  • unknown routes rendered as a bare Not Found string
  • no meaningful content container
  • no clear path back to the homepage

After

chrome-capture-2026-03-28
  • unknown routes render inside the normal app shell
  • the page has a styled 404 container consistent with the site
  • users get a single, obvious Return home action

Verification

  • bunx vitest run src/components/NotFoundPage.test.tsx src/router.test.tsx
  • bunx tsc --noEmit
  • bunx tsc -p packages/schema/tsconfig.json --noEmit
  • bunx tsc -p packages/clawdhub/tsconfig.json --noEmit
  • bun --bun vite build
  • verified locally in browser at /asdfd
  • verified with Playwright against the local page

This PR is assisted by codex.

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 28, 2026

@kevinw-openai is attempting to deploy a commit to the 0xBuns Team on Vercel.

A member of the Team first needs to authorize it.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Unknown error
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR replaces TanStack Router's bare default "Not Found" paragraph with a properly styled NotFoundPage component registered via defaultNotFoundComponent. The implementation is clean and consistent with the rest of the app:

  • NotFoundPage follows the same <main className=\"section\"> layout used by every other route component; the root shell (RootDocument) wraps it correctly without any nested <main> concerns.
  • New .not-found-* CSS classes use the existing design tokens (--surface, --accent-deep, --shadow, CSS custom properties for dark mode), clamp() sizing for responsiveness, and a mobile breakpoint consistent with the rest of styles.css.
  • Router registration (defaultNotFoundComponent: NotFoundPage) and the component render are both covered by tests.

One minor style note: in NotFoundPage.test.tsx line 20, screen.getByText(...) already throws when the element is absent, so .toBeTruthy() is redundant — toBeInTheDocument() or relying on the implicit throw is more idiomatic.

Confidence Score: 5/5

Safe to merge — no P0 or P1 issues; all findings are minor style suggestions.

The change is small, well-scoped, and well-tested. It follows existing patterns throughout the codebase (layout, styling, test structure) and has no correctness, data-integrity, or reliability concerns. The only finding is a redundant .toBeTruthy() assertion in a test (P2 style).

No files require special attention.

Important Files Changed

Filename Overview
src/components/NotFoundPage.tsx New component rendering a styled 404 page; follows existing <main className="section"> conventions used across all other route components.
src/components/NotFoundPage.test.tsx Tests render the component with a mocked Link and assert the 404 heading and "Return home" CTA; one redundant .toBeTruthy() assertion on getByText (P2 style).
src/router.tsx Imports and wires NotFoundPage as defaultNotFoundComponent; change is minimal and correct.
src/router.test.tsx Verifies the router option is set to the correct component reference; test structure and env setup look sound.
src/styles.css Adds well-scoped .not-found-* CSS classes consistent with the existing design tokens and responsive patterns; no conflicts with existing rules detected.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/components/NotFoundPage.test.tsx
Line: 20

Comment:
**Redundant `.toBeTruthy()` after `getByText`**

`screen.getByText(...)` already throws (and fails the test) when the element is not found — the chained `.toBeTruthy()` is always truthy on a successful query and adds no extra safety net. Prefer `toBeInTheDocument()` for a more expressive assertion, or simply omit the assertion and rely on `getByText`'s implicit throw:

```suggestion
    expect(screen.getByText("404 • Page not found")).toBeInTheDocument();
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: add a proper 404 page" | Re-trigger Greptile

it("renders a single recovery action back to the homepage", () => {
render(<NotFoundPage />);

expect(screen.getByText("404 • Page not found")).toBeTruthy();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant .toBeTruthy() after getByText

screen.getByText(...) already throws (and fails the test) when the element is not found — the chained .toBeTruthy() is always truthy on a successful query and adds no extra safety net. Prefer toBeInTheDocument() for a more expressive assertion, or simply omit the assertion and rely on getByText's implicit throw:

Suggested change
expect(screen.getByText("404 • Page not found")).toBeTruthy();
expect(screen.getByText("404 • Page not found")).toBeInTheDocument();
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/NotFoundPage.test.tsx
Line: 20

Comment:
**Redundant `.toBeTruthy()` after `getByText`**

`screen.getByText(...)` already throws (and fails the test) when the element is not found — the chained `.toBeTruthy()` is always truthy on a successful query and adds no extra safety net. Prefer `toBeInTheDocument()` for a more expressive assertion, or simply omit the assertion and rely on `getByText`'s implicit throw:

```suggestion
    expect(screen.getByText("404 • Page not found")).toBeInTheDocument();
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant