Skip to content

fix: improve error handling and add routing#41

Open
kunal-bunkar wants to merge 2 commits intoAOSSIE-Org:mainfrom
kunal-bunkar:feature/add-routing
Open

fix: improve error handling and add routing#41
kunal-bunkar wants to merge 2 commits intoAOSSIE-Org:mainfrom
kunal-bunkar:feature/add-routing

Conversation

@kunal-bunkar
Copy link

@kunal-bunkar kunal-bunkar commented Mar 12, 2026

Addressed Issues:

Fixes #40

Screenshots/Recordings:

  • Unknown routes now show a 404 page with a "Go home" link
image

Additional Notes:

This PR introduces minimal client-side routing using react-router-dom:

  • Wrapped the app with BrowserRouter in src/main.tsx
  • Added route definitions in src/App.tsx for:
    • / (home)
    • * (404)
  • Extracted the 404 UI into a separate component file src/NotFoundPage.tsx

Verification:

  • npm run lint passes

Checklist

  • My code follows the project's code style and conventions
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have joined the Discord server and I will share a link to this PR with the project maintainers there
  • [x ] I have read the Contributing Guidelines

Summary by CodeRabbit

  • New Features
    • Added client-side routing so users can navigate between pages within the app.
    • Added a 404 "Not Found" page that shows when users visit undefined routes, with a clear link to return home.

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 90f338da-0c2a-45d2-8d78-66beaa6f280b

📥 Commits

Reviewing files that changed from the base of the PR and between 944a05b and fb995e5.

📒 Files selected for processing (2)
  • src/App.tsx
  • src/NotFoundPage.tsx

Walkthrough

Adds react-router-dom, wraps the app with BrowserRouter, defines Routes in App ("/" → Home UI, "*" → NotFoundPage), and introduces a new NotFoundPage component with a link back to root.

Changes

Cohort / File(s) Summary
Manifest / Dependency
package.json
Added dependency react-router-dom (^7.13.1) and adjusted JSON formatting for the new entry.
App Routing
src/App.tsx
Replaced direct render with <Routes> and two routes: / (internal HomeRoute) and * (imports and renders NotFoundPage).
Router Context
src/main.tsx
Wrapped <App /> with <BrowserRouter> to provide routing context.
404 Component
src/NotFoundPage.tsx
New default-exported NotFoundPage component rendering a 404 message and a Link back to /.

Sequence Diagram(s)

sequenceDiagram
    participant User as "Client (Browser)"
    participant Router as "BrowserRouter"
    participant App as "App"
    participant Home as "HomeRoute"
    participant NotFound as "NotFoundPage"

    User->>Router: Navigate to URL
    Router->>App: Provide routing context + current path
    alt path == "/"
        App->>Home: Render HomeRoute
        Home-->>User: Send Home UI
    else unknown path
        App->>NotFound: Render NotFoundPage
        NotFound-->>User: Send 404 + Link to "/"
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Typescript Lang

Poem

🐰 I hopped through routes both near and far,
"/" was bright — a guiding star.
If you wander off the mapped-out way,
A friendly 404 will show the way home today. 🏠

🚥 Pre-merge checks | ✅ 4
✅ 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 identifies the main change: adding routing capability to the application, which aligns with the primary objective of implementing client-side routing.
Linked Issues check ✅ Passed All coding requirements from issue #40 are met: BrowserRouter wrapping in main.tsx, route definitions for '/' and '*' in App.tsx, NotFoundPage.tsx created, and routing functionality fully implemented.
Out of Scope Changes check ✅ Passed All changes are directly related to the routing feature: react-router-dom dependency added, BrowserRouter implementation, route definitions, and 404 page creation align with issue #40 objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@github-actions github-actions bot added size/M and removed size/M labels Mar 12, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/App.tsx`:
- Around line 9-10: The component currently hardcodes user-visible strings
(e.g., "Hello, OrgExplorer!", "404", "Page not found.", "Go home") inside the
App component and route JSX; extract these literals into the i18n/resource
system and replace the inline text with lookups (e.g., use t('home.title') or a
resources object) within App and any route components (refer to App / the Route
rendering that shows "404"/"Page not found."/"Go home"). Ensure keys are added
to the resource files and used via the existing i18n/getString utility or a new
simple wrapper so all visible strings in the component tree are read from
resources rather than hardcoded.
- Around line 16-24: The NotFoundRoute component is implemented inline in App as
function NotFoundRoute; extract it into a new file src/NotFoundPage.tsx as a
default-exported React component (e.g., export default function NotFoundPage())
that returns the same JSX (h1, p, Link from 'react-router-dom'), then update App
to import NotFoundPage and replace references to NotFoundRoute with the imported
NotFoundPage (and remove the original NotFoundRoute function from App).

In `@src/main.tsx`:
- Around line 9-11: The app currently uses BrowserRouter in main.tsx (wrapping
<App />) which requires the hosting to rewrite unknown paths to index.html for
direct navigations; verify the deployment configuration provides that SPA
fallback (rewrite/redirect unknown routes to index.html) and confirm in the PR,
or if the host cannot supply rewrites switch to HashRouter (imported from
react-router-dom and replace BrowserRouter with HashRouter around <App />) so
hard-refreshes/direct requests work without server rewrites.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 614d2e12-ba5d-47dd-97e1-4e847b467dfd

📥 Commits

Reviewing files that changed from the base of the PR and between 1cfc3e7 and 944a05b.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • package.json
  • src/App.tsx
  • src/main.tsx

src/App.tsx Outdated
Comment on lines +9 to +10
<h1>Hello, OrgExplorer!</h1>
</>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Externalize the routed page copy.

Hello, OrgExplorer!, 404, Page not found., and Go home are hardcoded in the component tree. Please move them into the project's resource files before expanding the route surface.

As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)."

Also applies to: 19-21

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 9 - 10, The component currently hardcodes
user-visible strings (e.g., "Hello, OrgExplorer!", "404", "Page not found.", "Go
home") inside the App component and route JSX; extract these literals into the
i18n/resource system and replace the inline text with lookups (e.g., use
t('home.title') or a resources object) within App and any route components
(refer to App / the Route rendering that shows "404"/"Page not found."/"Go
home"). Ensure keys are added to the resource files and used via the existing
i18n/getString utility or a new simple wrapper so all visible strings in the
component tree are read from resources rather than hardcoded.

Comment on lines +9 to +11
<BrowserRouter>
<App />
</BrowserRouter>
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Verify SPA fallback support for BrowserRouter.

This setup only handles misses after the client boots. A direct request or hard refresh on /xyz will still return a server 404 unless the deployed host rewrites unknown paths to index.html. Please confirm that rewrite exists, or use HashRouter if the target hosting cannot provide it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main.tsx` around lines 9 - 11, The app currently uses BrowserRouter in
main.tsx (wrapping <App />) which requires the hosting to rewrite unknown paths
to index.html for direct navigations; verify the deployment configuration
provides that SPA fallback (rewrite/redirect unknown routes to index.html) and
confirm in the PR, or if the host cannot supply rewrites switch to HashRouter
(imported from react-router-dom and replace BrowserRouter with HashRouter around
<App />) so hard-refreshes/direct requests work without server rewrites.

@github-actions github-actions bot added size/M and removed size/M labels Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FEATURE : Add basic routing and separate 404 page (React Router)

1 participant