Skip to content

Add React Router support with 404 page #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
coderpurohit wants to merge 1 commit into from
+141 −28
Closed

Add React Router support with 404 page #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
77 changes: 74 additions & 3 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
},
"dependencies": {
"react": "^19.2.0",
"react-dom": "^19.2.0"
"react-dom": "^19.2.0",
"react-router-dom": "^7.15.0"
},
"devDependencies": {
"@eslint/js": "^9.39.1",
Expand Down
31 changes: 28 additions & 3 deletions src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
#root {
max-width: 1280px;
margin: 0 auto;
padding: 2rem;
width: 100%;
}

.home-container,
.notfound-container {
min-height: 100vh;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use dynamic viewport height to avoid mobile clipping/jump.

min-height: 100vh can produce layout jumps on mobile browser UI changes. Add 100dvh with fallback.

Proposed fix 
 .home-container,
 .notfound-container {
-  min-height: 100vh;
+  min-height: 100vh;
+  min-height: 100dvh;

As per coding guidelines, "**/*.css: ...adhere to best practices recommended by lighthouse or similar tools for performance."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
min-height: 100vh;
.home-container,
.notfound-container {
min-height: 100vh;
min-height: 100dvh;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.css` at line 7, Replace the static viewport height declaration in
App.css (the min-height: 100vh property) with a dynamic viewport fallback
pattern to avoid mobile clipping: keep the legacy fallback first (min-height:
100vh) and then override it with the dynamic unit (min-height: 100dvh) so modern
browsers use 100dvh while older ones fall back to 100vh.


display: flex;
flex-direction: column;

justify-content: center;
align-items: center;

text-align: center;
}
Comment on lines +5 to +16
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix stylelint violations in the route container block.

Line 9, Line 12, and Line 15 have empty lines before declarations (declaration-empty-line-before), which will fail stylelint.

Proposed fix 
 .home-container,
 .notfound-container {
   min-height: 100vh;
-
   display: flex;
   flex-direction: column;
-
   justify-content: center;
   align-items: center;
-
   text-align: center;
 }

As per coding guidelines, "**/*.css: Review the CSS code against the google css style guide... and best practices associated with CSS."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.home-container,
.notfound-container {
min-height: 100vh;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
text-align: center;
}
.home-container,
.notfound-container {
min-height: 100vh;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
text-align: center;
}
🧰 Tools
🪛 Stylelint (17.11.0)

[error] 9-9: Expected no empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)

[error] 12-12: Expected no empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)

[error] 15-15: Expected no empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.css` around lines 5 - 16, The .home-container, .notfound-container
rule contains unnecessary blank lines before declarations which triggers
stylelint's declaration-empty-line-before; remove the empty lines so
declarations (min-height, display, flex-direction, justify-content, align-items,
text-align) are directly one-per-line without preceding blank lines and preserve
existing ordering and indentation in the .home-container, .notfound-container
block to satisfy the CSS style guide.


h1 {
font-size: 4rem;
margin-bottom: 20px;
}

a {
color: #4ea1ff;
text-decoration: none;
font-size: 18px;
}

a:hover {
text-decoration: underline;
}
Comment on lines +18 to 31
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Scope typography/link rules to route containers to prevent global style bleed.

These global selectors can unintentionally restyle headings/links in unrelated views.

Proposed refactor 
-h1 {
+.home-container h1,
+.notfound-container h1 {
   font-size: 4rem;
   margin-bottom: 20px;
 }
 
-a {
+.home-container a,
+.notfound-container a {
   color: `#4ea1ff`;
   text-decoration: none;
   font-size: 18px;
 }
 
-a:hover {
+.home-container a:hover,
+.notfound-container a:hover {
   text-decoration: underline;
 }

As per coding guidelines, "**/*.css: ...adhere to similar naming conventions for classes, ids."

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.css` around lines 18 - 31, The global selectors h1, a, and a:hover
are leaking styles; scope them to route/container classes instead (e.g., use a
route wrapper class like .route-container or specific page classes) so headings
and links in other views aren't affected; update the rules that target h1, a,
and a:hover to target .route-container h1, .route-container a, and
.route-container a:hover (or the specific route/page class names you use) and
ensure class names follow the project's naming conventions for CSS files.

28 changes: 22 additions & 6 deletions src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
import './App.css'
import { Routes, Route } from 'react-router-dom'
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n src/App.tsx

Repository: AOSSIE-Org/OrgExplorer

Length of output: 694

Use React Router Link instead of native anchor, and externalize hardcoded UI strings.

Using <a href="/"> triggers a full page reload in this React Router SPA. Additionally, user-visible strings ("Hello, OrgExplorer!", "404 - Page Not Found", "Go Back Home") are hardcoded and should be externalized to i18n resource files per coding guidelines.

Suggested fix 
-import { Routes, Route } from 'react-router-dom'
+import { Routes, Route, Link } from 'react-router-dom'
@@
-      <a href="/">Go Back Home</a>
+      <Link to="/">Go Back Home</Link>

Also externalize hardcoded strings to i18n resources.

Also applies to: 13-13 (and strings on lines 5, 12)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.tsx` at line 1, Replace native anchors with React Router's Link: add
import { Link } from 'react-router-dom' and change <a href="/"> usages to <Link
to="/"> to avoid full page reloads (search for the anchor in App.tsx).
Externalize the hardcoded UI strings "Hello, OrgExplorer!", "404 - Page Not
Found", and "Go Back Home" to the i18n resource files and replace usages with
the project i18n lookup (e.g., useTranslation hook or existing t function) so
the component reads labels via t('...') instead of string literals; update
imports to include the i18n hook (useTranslation or project equivalent) and use
the same translation keys in the resource files.


function App() {
function Home() {
return (
<h1>Hello, OrgExplorer!</h1>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Externalize user-facing strings to i18n resources.

These hardcoded strings should come from localization resources to meet the repo’s i18n requirement.

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

Also applies to: 12-13

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.tsx` at line 5, Replace hardcoded user-facing strings in the App
component with i18n lookups: import and use the translation hook (e.g.,
useTranslation or t) in the App function and replace occurrences like
"<h1>Hello, OrgExplorer!</h1>" and the strings on lines 12–13 with t('key')
calls (define keys such as "app.title" and the other two keys in your locale
resource files). Ensure you add the new keys to the appropriate locale JSON and
keep the original text as the default value in the resource file.

)
}

function NotFoundPage() {
return (
<div>
<h1>404 - Page Not Found</h1>
<a href="/">Go Back Home</a>
</div>
)
}

function App() {
return (
<>
<h1>Hello, OrgExplorer!</h1>
</>
<Routes>
<Route path="/" element={<Home />} />

<Route path="*" element={<NotFoundPage />} />
</Routes>
)
}

export default App
export default App
22 changes: 9 additions & 13 deletions src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
:root {
font-family: system-ui, Avenir, Helvetica, Arial, sans-serif;
line-height: 1.5;
font-weight: 400;

color-scheme: light dark;
color: rgba(255, 255, 255, 0.87);
background-color: #242424;

font-synthesis: none;
text-rendering: optimizeLegibility;
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
* {
margin: 0;
padding: 0;
box-sizing: border-box;
}

body {
font-family: Arial, sans-serif;
background-color: #111;
color: white;
}
8 changes: 6 additions & 2 deletions src/main.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { StrictMode } from 'react'
import { createRoot } from 'react-dom/client'
import { BrowserRouter } from 'react-router-dom'

import './index.css'
import App from './App.tsx'

createRoot(document.getElementById('root')!).render(
<StrictMode>
<App />
<BrowserRouter>
<App />
</BrowserRouter>
</StrictMode>,
)
)
Loading