feat: improve SEO/a11y, add Contact route, env API URL, 404 page, rob…#133
feat: improve SEO/a11y, add Contact route, env API URL, 404 page, rob…#133MananGilhotra wants to merge 1 commit intoOpen-Source-Kashmir:mainfrom
Conversation
…ots/sitemap, backend health + env CORS/PORT
WalkthroughThe PR adds environment-driven API configuration, new frontend routes for Contact and 404 pages, backend health check and configurable CORS support, SEO improvements via robots.txt and sitemap, and HTML accessibility enhancements. It establishes prerequisites for development setup including required .env configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrontendApp as Frontend App
participant Env as .env / Environment
participant Backend as Backend (contact.js)
User->>FrontendApp: Navigates to /contact
FrontendApp->>Env: Reads VITE_API_BASE_URL
Env-->>FrontendApp: Returns API base URL (or default http://localhost:5002)
FrontendApp->>Backend: POST /api/contact (via configured baseUrl)
note over Backend: CORS check using<br/>process.env.CORS_ORIGIN
Backend-->>FrontendApp: 200 Success
FrontendApp-->>User: Form submitted
User->>Backend: GET /health (health check)
Backend-->>User: { status: 'ok' } 200
User->>FrontendApp: Accesses invalid route
FrontendApp->>FrontendApp: Catch-all "*" route triggers
FrontendApp-->>User: Renders NotFound 404 page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Contact_Form/Contact.jsx (1)
20-21: Remove debug console.log statements before production.Multiple debug
console.logstatements are present throughout the form submission handler. These should be removed or replaced with a proper logging solution for production.Consider:
- Removing all debug logs (lines 20, 21, 28, 35, 36, 39, 42, 46, 55)
- Keeping only the
console.erroron line 50 for error tracking- Or using a conditional logging approach:
const log = import.meta.env.DEV ? console.log : () => {}; // Then use log() instead of console.log() log('Form submission started');Also applies to: 28-28, 35-36, 39-39, 42-42, 46-46, 55-55
backend/contact.js (1)
33-88: Add rate limiting to prevent abuse.The
/api/contactendpoint has no rate limiting, making it vulnerable to spam and abuse. Consider adding rate limiting middleware.Install and configure rate limiting:
npm install express-rate-limitThen apply rate limiting:
const express = require('express'); const nodemailer = require('nodemailer'); const cors = require('cors'); +const rateLimit = require('express-rate-limit'); require('dotenv').config(); const app = express(); const PORT = process.env.PORT || 5002; // CORS setup const allowedOrigin = process.env.CORS_ORIGIN || 'http://localhost:5173'; app.use(cors({ origin: allowedOrigin, methods: ['GET', 'POST', 'OPTIONS'], allowedHeaders: ['Content-Type'], })); app.use(express.json()); +// Rate limiter for contact form +const contactLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 5, // limit each IP to 5 requests per windowMs + message: 'Too many contact form submissions, please try again later.', +}); // Health check app.get('/health', (_req, res) => { res.status(200).json({ status: 'ok' }); }); // ... transporter setup ... // POST /api/contact -app.post('/api/contact', async (req, res) => { +app.post('/api/contact', contactLimiter, async (req, res) => {
🧹 Nitpick comments (5)
README.md (2)
74-80: Add guidance on.envfile safety and provide cross-platform instructions.The new frontend environment setup documentation is helpful and well-placed, but the
echocommand approach has a few gaps:
- No mention that
.envshould be added to.gitignoreto prevent accidental commits of sensitive/local config.- The
echocommand will silently overwrite an existing.envfile without warning.- Cross-platform considerations—Windows users may need different instructions.
Consider revising to:
6. Configure frontend API base URL Create a frontend env var so the contact form knows where to send requests: ```bash # create a .env file in the project root (same level as package.json) + # (Make sure .env is in .gitignore and never committed) - echo "VITE_API_BASE_URL=http://localhost:5002" > .env + cat > .env << 'EOF' + VITE_API_BASE_URL=http://localhost:5002 + EOF ```Alternatively, consider creating a
.env.exampletemplate file in the repo that users can copy:cp .env.example .envThis approach is more maintainable and makes future env vars discoverable.
74-80: Clarify that frontend.envsetup is required before runningnpm run dev.The ordering and integration are good, but the documentation could explicitly state that Step 6 is a prerequisite for Step 7. Consider adding a note like:
⚠️ Note: Step 6 must be completed before running the frontend development server, otherwise the contact form will not be able to reach the API.This prevents users from skipping the configuration step and running into confusing errors.
public/sitemap.xml (1)
3-24: Add optional sitemap metadata for improved SEO signals.While the sitemap is structurally valid, adding
<lastmod>and<changefreq>elements would improve search engine crawl efficiency and freshness signals. Currently, only the root URL includes any metadata (priority).Consider enhancing entries with:
<lastmod>YYYY-MM-DD</lastmod>– helps search engines know when content was last updated<changefreq>weekly|monthly</changefreq>– hints how often each page changesThis is optional but follows sitemap best practices.
Example enhancement for one entry:
<url> <loc>https://osk-open-source.netlify.app/programs</loc> + <lastmod>2025-10-29</lastmod> + <changefreq>monthly</changefreq> </url>src/components/Contact_Form/Contact.jsx (1)
27-29: Consider moving API base URL outside the handler.The
apiBaseUrlis derived from the environment on every form submission. For efficiency, consider moving it outside thehandleSubmitfunction.Apply this diff:
+const apiBaseUrl = import.meta.env.VITE_API_BASE_URL || 'http://localhost:5002'; + export default function Contact() { const [formData, setFormData] = useState({ name: '', email: '', subject: '', message: '' }); const [status, setStatus] = useState(''); const [loading, setLoading] = useState(false); const handleChange = (e) => setFormData({ ...formData, [e.target.name]: e.target.value }); const handleSubmit = async (e) => { e.preventDefault(); console.log('Form submission started'); console.log('Form data:', formData); setLoading(true); setStatus('Sending...'); try { - const apiBaseUrl = import.meta.env.VITE_API_BASE_URL || 'http://localhost:5002'; console.log('Sending request to backend...'); const response = await fetch(`${apiBaseUrl}/api/contact`, {backend/contact.js (1)
19-22: Consider enhancing the health check endpoint.The current health check simply returns
{ status: 'ok' }without verifying dependencies. For production use, consider checking critical services like the email transporter.Example enhancement:
app.get('/health', async (_req, res) => { const health = { status: 'ok', timestamp: Date.now(), uptime: process.uptime(), }; // Optional: verify email service try { await transporter.verify(); health.emailService = 'ok'; } catch (error) { health.emailService = 'degraded'; health.status = 'degraded'; } const statusCode = health.status === 'ok' ? 200 : 503; res.status(statusCode).json(health); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)backend/contact.js(1 hunks)index.html(1 hunks)public/robots.txt(1 hunks)public/sitemap.xml(1 hunks)src/App.jsx(3 hunks)src/components/Contact_Form/Contact.jsx(1 hunks)src/components/Navbar.jsx(2 hunks)src/pages/NotFound.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/App.jsx (2)
src/components/Contact_Form/Contact.jsx (1)
Contact(6-225)src/pages/NotFound.jsx (1)
NotFound(3-19)
🔇 Additional comments (9)
public/robots.txt (1)
4-4: Hardcoded domain in both robots.txt and sitemap.xml limits deployment flexibility; refactor to use environment variables.Both
robots.txtandpublic/sitemap.xmlare hardcoded tohttps://osk-open-source.netlify.app/. While the project demonstrates environment-aware patterns elsewhere (e.g.,VITE_API_BASE_URLin Contact.jsx), the domain configuration is inconsistent. If the site is deployed to staging, preview, or a different domain, both files would become incorrect and degrade SEO effectiveness.Recommended approach:
- Add a
VITE_PUBLIC_DOMAINenvironment variable (e.g.,.env)- Use build-time substitution to inject the domain into both robots.txt and sitemap.xml during the build process, or
- Generate sitemap.xml dynamically at build time using the environment variable
This aligns with the project's existing environment configuration pattern.
public/sitemap.xml (1)
1-26: Verify multi-environment deployment strategy before addressing hardcoded domains.The hardcoded domain issue does exist across three files (robots.txt, sitemap.xml, and index.html). However, the codebase shows no evidence of multi-environment deployment setup—no netlify.toml, .env.example, or staging/dev configuration. While Vite supports environment variables natively (and the project already uses them for backend API URL), whether this concern applies depends on your actual deployment needs.
Confirm: Do you need staging, development, or other non-production deployments? If not, these hardcoded URLs are appropriate for a production-only project. If yes, implement environment-based URL injection for robots.txt and sitemap.xml (and update index.html's og:url and canonical similarly).
index.html (1)
32-32: Excellent accessibility implementation!The skip-to-content link is properly implemented with screen-reader-only styling and focus visibility. It correctly targets
#main-contentwhich exists in src/App.jsx (line 35).backend/contact.js (1)
7-7: Good environment-driven configuration.The configurable PORT and CORS_ORIGIN with sensible defaults align well with the frontend changes. The default port (5002) matches the frontend fallback, and the default origin matches Vite's dev server.
If your app will use cookies or authentication, verify whether you need to add the
credentialsoption to CORS:app.use(cors({ origin: allowedOrigin, methods: ['GET', 'POST', 'OPTIONS'], allowedHeaders: ['Content-Type'], credentials: true, // if using cookies/auth }));Also applies to: 10-10, 12-12
src/components/Navbar.jsx (1)
72-78: LGTM! Consistent navigation implementation.The Contact link is properly added to both desktop and mobile menus with consistent styling, active state handling, and menu-close behavior matching existing navigation items.
Also applies to: 193-199
src/pages/NotFound.jsx (1)
3-19: Clean and user-friendly 404 page.The NotFound component provides a clear error message with helpful navigation options. The implementation is simple and effective.
src/App.jsx (3)
16-17: LGTM! Proper imports for new routes.The Contact and NotFound components are correctly imported and ready for routing.
35-35: Excellent accessibility support.The
id="main-content"attribute properly supports the skip-to-content link added in index.html, enabling keyboard users to bypass navigation.
45-46: Routes are properly configured and dependencies are compatible.React 19.1.1 is compatible with react-router-dom 7.9.3, as React Router v7 was designed to support both React 18 and React 19. The
/contactroute and catch-all*route implementation will work correctly under these versions.
| <link rel="icon" href="/favicon.png" type="image/png" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <link rel="canonical" href="https://osk-open-source.netlify.app/" /> |
There was a problem hiding this comment.
Canonical URL should be dynamic per route.
The hardcoded canonical URL will incorrectly apply to all routes (/contact, /programs, etc.), causing all pages to declare the homepage as canonical. This harms SEO by preventing proper indexing of individual pages.
Consider using a dynamic solution:
- For React apps, use a library like
react-helmetorreact-helmet-asyncto set canonical URLs per route. - Alternatively, handle this at build/deployment time if using SSG/SSR.
Example with react-helmet:
// In each page component
import { Helmet } from 'react-helmet-async';
<Helmet>
<link rel="canonical" href={`https://osk-open-source.netlify.app${location.pathname}`} />
</Helmet>🤖 Prompt for AI Agents
In index.html around line 27 the canonical link is hardcoded to the homepage
which makes every route report the same canonical URL; remove this static tag
from the global HTML and instead generate a per-route canonical URL at runtime
or build time — for a React app add canonical tags in each page component (e.g.,
via react-helmet or react-helmet-async) using the current pathname to build the
absolute URL, or if using SSG/SSR emit the canonical during page generation;
ensure the canonical is the full absolute URL and handle trailing slashes/params
consistently.
|
Please attach a screen recording of the website working |
|
Please merge my pr please
… On 29 Oct 2025, at 11:29 PM, Athar Ramzan ***@***.***> wrote:
Please attach a screen recording of the website working
|
adds canonical URL and theme-color meta, removes duplicate viewport tag, and includes a “Skip to content” link for better SEO and a11y.
Introduces a friendly 404 page and a catch-all route to guide users when a path doesn’t exist.
Adds robots.txt and sitemap.xml to help search engines crawl and index key pages properly
Defaults PORT to 5002, reads CORS_ORIGIN from env, and adds a /health endpoint for simple uptime checks and safer defaults
Summary by CodeRabbit
New Features
Documentation
Chores