-
Notifications
You must be signed in to change notification settings - Fork 144
Add global footer component and remove duplicate footers #266
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.
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a new global Footer component rendered from Changes
Sequence DiagramsequenceDiagram
participant App as App.tsx
participant Footer as Footer Component
participant Observer as IntersectionObserver
participant DOM as DOM
App->>Footer: render Footer
activate Footer
Footer->>DOM: mount footer element
Footer->>Observer: create observer (threshold 0.15)
activate Observer
Note over Observer,Footer: Observer watches footer visibility
alt footer enters viewport
Observer->>Footer: intersection callback
Footer->>DOM: apply fade/slide-in animation
else footer not visible
Observer->>Footer: no intersection
end
Footer->>Observer: cleanup on unmount
deactivate Observer
deactivate Footer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
Frontend/src/pages/HomePage.tsx (1)
429-430: Remove or update outdated comment about footer observer.The comment references keeping "other observers for footer" but the footer observer and all footer-related code have been removed from this component. This comment is now misleading.
🔎 Proposed fix
- // ... keep other observers for footer, etc. if needed ...Or update it to reflect current observers:
- // ... keep other observers for footer, etc. if needed ... + // Intersection observers for section animations
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
Frontend/package.jsonFrontend/src/App.tsxFrontend/src/components/footer.tsxFrontend/src/pages/HomePage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/App.tsx (1)
Frontend/src/context/AuthContext.tsx (1)
AuthProvider(64-215)
🔇 Additional comments (4)
Frontend/src/App.tsx (2)
48-50: Layout structure looks good.The flex-col container ensures the footer stays at the bottom while allowing the main content to grow. This is a clean implementation of the sticky footer pattern.
Also applies to: 140-145
23-23: [Your rewritten review comment text here]
[Exactly ONE classification tag]Frontend/src/components/footer.tsx (1)
1-3: Imports are clean and minimal.Good use of only the necessary dependencies. The component doesn't rely on the newly added Material UI libraries.
Frontend/src/pages/HomePage.tsx (1)
386-393: Footer removal implemented cleanly.The footer-related code (refs, state, useEffect, and JSX) has been properly removed from HomePage, successfully eliminating duplication. The component now relies on the global Footer rendered in App.tsx.
| "@emotion/react": "^11.14.0", | ||
| "@emotion/styled": "^11.14.1", | ||
| "@mui/icons-material": "^7.3.6", | ||
| "@mui/material": "^7.3.6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Material UI imports across the codebase
echo "=== Checking for @mui/material imports ==="
rg -n "from ['\"]@mui/material" --type=tsx --type=ts --type=jsx --type=js -C2
echo -e "\n=== Checking for @mui/icons-material imports ==="
rg -n "from ['\"]@mui/icons-material" --type=tsx --type=ts --type=jsx --type=js -C2
echo -e "\n=== Checking for @emotion imports ==="
rg -n "from ['\"]@emotion/(react|styled)" --type=tsx --type=ts --type=jsx --type=js -C2
echo -e "\n=== Checking MUI imports (alternative patterns) ==="
rg -n "@mui|@emotion" --type=tsx --type=ts --type=jsx --type=js | head -30Repository: AOSSIE-Org/InPactAI
Length of output: 376
🏁 Script executed:
#!/bin/bash
# Search for Material UI and Emotion imports without type restrictions
echo "=== Checking for @mui/material imports ==="
rg "@mui/material" Frontend/ -A2
echo -e "\n=== Checking for @mui/icons-material imports ==="
rg "@mui/icons-material" Frontend/ -A2
echo -e "\n=== Checking for @emotion imports ==="
rg "@emotion/(react|styled)" Frontend/ -A2
echo -e "\n=== Checking footer.tsx content ==="
if [ -f "Frontend/src/components/footer.tsx" ]; then
cat -n Frontend/src/components/footer.tsx
else
echo "footer.tsx not found at expected location"
find Frontend -name "footer.tsx" -o -name "footer.ts"
fi
echo -e "\n=== All files in Frontend/src/components ==="
find Frontend/src/components -type f -name "*.tsx" -o -name "*.ts" 2>/dev/null | head -20Repository: AOSSIE-Org/InPactAI
Length of output: 10334
🏁 Script executed:
#!/bin/bash
# More comprehensive search for MUI/Emotion usage in all source files
echo "=== All TypeScript/JavaScript source files searching for MUI usage ==="
find Frontend/src -type f \( -name "*.tsx" -o -name "*.ts" -o -name "*.jsx" -o -name "*.js" \) | while read file; do
if grep -l "@mui\|@emotion" "$file" 2>/dev/null; then
echo "Found in: $file"
grep -n "@mui\|@emotion" "$file"
fi
done
echo -e "\n=== Checking all files for import statements from @mui or @emotion ==="
rg "import.*from.*[@'\"]@(mui|emotion)" Frontend/src/ 2>/dev/null || echo "No matches found"
echo -e "\n=== Checking all files for require statements from @mui or @emotion ==="
rg "require.*[@'\"]@(mui|emotion)" Frontend/src/ 2>/dev/null || echo "No matches found"Repository: AOSSIE-Org/InPactAI
Length of output: 312
Remove unused Material UI and Emotion dependencies.
MUI and Emotion packages (@mui/material, @mui/icons-material, @emotion/react, @emotion/styled) are declared in package.json but have zero imports throughout the entire codebase. The Footer component uses lucide-react for icons and Tailwind CSS for styling—no MUI components are utilized.
These libraries add ~500KB+ combined minified size with no benefit. Remove them from package.json and run npm install to clean up the dependency tree.
🤖 Prompt for AI Agents
In Frontend/package.json around lines 13 to 16, remove the unused dependencies
"@mui/material", "@mui/icons-material", "@emotion/react", and "@emotion/styled"
from the dependencies list, then run npm install (or npm ci) to update
node_modules and package-lock.json; before removing, ensure a repo-wide search
confirms there are no imports/usages of these packages (the review indicates
zero imports), and after install run the build and a quick smoke test to verify
nothing breaks.
| useEffect(() => { | ||
| const observer = new IntersectionObserver( | ||
| ([entry]) => { | ||
| setIsFooterVisible(entry.isIntersecting); | ||
| }, | ||
| { threshold: 0.15 } | ||
| ); | ||
|
|
||
| if (footerRef.current) observer.observe(footerRef.current); | ||
|
|
||
| return () => { | ||
| if (footerRef.current) observer.unobserve(footerRef.current); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix potential memory leak in IntersectionObserver cleanup.
The cleanup function accesses footerRef.current which may be null or may have changed by the time cleanup runs. In React 19, refs can be cleaned up before effects, leading to potential issues.
🔎 Proposed fix
useEffect(() => {
const observer = new IntersectionObserver(
([entry]) => {
setIsFooterVisible(entry.isIntersecting);
},
{ threshold: 0.15 }
);
- if (footerRef.current) observer.observe(footerRef.current);
+ const element = footerRef.current;
+ if (element) observer.observe(element);
return () => {
- if (footerRef.current) observer.unobserve(footerRef.current);
+ if (element) observer.unobserve(element);
+ observer.disconnect();
};
}, []);🤖 Prompt for AI Agents
In Frontend/src/components/footer.tsx around lines 9 to 22, the cleanup
currently reads footerRef.current which can be null or changed before cleanup;
capture the observed element in a local variable (e.g., const observed =
footerRef.current) before calling observer.observe, then in the cleanup check
and call observer.unobserve(observed) only if observed is non-null and finally
call observer.disconnect() to fully clean up the observer; ensure you reference
the captured element rather than footerRef.current in the cleanup to avoid the
potential memory leak.
| <nav className="mt-8 flex flex-wrap justify-center gap-x-8 gap-y-3 text-sm text-gray-400"> | ||
| <Link to="/dashboard" className="hover:text-white transition-colors"> | ||
| Dashboard | ||
| </Link> | ||
| <Link | ||
| to="/dashboard/sponsorships" | ||
| className="hover:text-white transition-colors" | ||
| > | ||
| Opportunities | ||
| </Link> | ||
| <Link | ||
| to="/dashboard/analytics" | ||
| className="hover:text-white transition-colors" | ||
| > | ||
| Analytics | ||
| </Link> | ||
| <Link | ||
| to="/dashboard/messages" | ||
| className="hover:text-white transition-colors" | ||
| > | ||
| Messages | ||
| </Link> | ||
| </nav> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Navigation links reference protected routes that unauthenticated users cannot access.
The footer includes links to /dashboard, /dashboard/sponsorships, /dashboard/analytics, and /dashboard/messages. Since this footer is rendered globally (including on public pages per App.tsx), unauthenticated users will see these links but encounter ProtectedRoute guards when clicking them.
Consider one of these approaches:
- Conditional rendering - Show different links based on authentication state
- Public links only - Use public navigation items (About, Pricing, Contact)
- Route-based footer - Render different footers for authenticated vs public routes
🔎 Example: Conditional navigation based on auth state
+import { useAuth } from "../context/AuthContext";
+
const Footer = () => {
const footerRef = useRef<HTMLElement | null>(null);
const [isFooterVisible, setIsFooterVisible] = useState(false);
+ const { isAuthenticated } = useAuth();
// ... useEffect ...
return (
<footer /* ... */>
{/* ... */}
<nav className="mt-8 flex flex-wrap justify-center gap-x-8 gap-y-3 text-sm text-gray-400">
- <Link to="/dashboard" className="hover:text-white transition-colors">
- Dashboard
- </Link>
- <Link to="/dashboard/sponsorships" className="hover:text-white transition-colors">
- Opportunities
- </Link>
- <Link to="/dashboard/analytics" className="hover:text-white transition-colors">
- Analytics
- </Link>
- <Link to="/dashboard/messages" className="hover:text-white transition-colors">
- Messages
- </Link>
+ {isAuthenticated ? (
+ <>
+ <Link to="/dashboard" className="hover:text-white transition-colors">
+ Dashboard
+ </Link>
+ <Link to="/dashboard/sponsorships" className="hover:text-white transition-colors">
+ Opportunities
+ </Link>
+ <Link to="/dashboard/analytics" className="hover:text-white transition-colors">
+ Analytics
+ </Link>
+ <Link to="/dashboard/messages" className="hover:text-white transition-colors">
+ Messages
+ </Link>
+ </>
+ ) : (
+ <>
+ <Link to="/" className="hover:text-white transition-colors">
+ Home
+ </Link>
+ <a href="#features" className="hover:text-white transition-colors">
+ Features
+ </a>
+ <a href="#pricing" className="hover:text-white transition-colors">
+ Pricing
+ </a>
+ <Link to="/login" className="hover:text-white transition-colors">
+ Login
+ </Link>
+ </>
+ )}
</nav>
{/* ... */}
</footer>
);
};📝 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.
| <nav className="mt-8 flex flex-wrap justify-center gap-x-8 gap-y-3 text-sm text-gray-400"> | |
| <Link to="/dashboard" className="hover:text-white transition-colors"> | |
| Dashboard | |
| </Link> | |
| <Link | |
| to="/dashboard/sponsorships" | |
| className="hover:text-white transition-colors" | |
| > | |
| Opportunities | |
| </Link> | |
| <Link | |
| to="/dashboard/analytics" | |
| className="hover:text-white transition-colors" | |
| > | |
| Analytics | |
| </Link> | |
| <Link | |
| to="/dashboard/messages" | |
| className="hover:text-white transition-colors" | |
| > | |
| Messages | |
| </Link> | |
| </nav> | |
| import { useAuth } from "../context/AuthContext"; | |
| const Footer = () => { | |
| const footerRef = useRef<HTMLElement | null>(null); | |
| const [isFooterVisible, setIsFooterVisible] = useState(false); | |
| const { isAuthenticated } = useAuth(); | |
| // ... useEffect ... | |
| return ( | |
| <footer /* ... */> | |
| {/* ... */} | |
| <nav className="mt-8 flex flex-wrap justify-center gap-x-8 gap-y-3 text-sm text-gray-400"> | |
| {isAuthenticated ? ( | |
| <> | |
| <Link to="/dashboard" className="hover:text-white transition-colors"> | |
| Dashboard | |
| </Link> | |
| <Link to="/dashboard/sponsorships" className="hover:text-white transition-colors"> | |
| Opportunities | |
| </Link> | |
| <Link to="/dashboard/analytics" className="hover:text-white transition-colors"> | |
| Analytics | |
| </Link> | |
| <Link to="/dashboard/messages" className="hover:text-white transition-colors"> | |
| Messages | |
| </Link> | |
| </> | |
| ) : ( | |
| <> | |
| <Link to="/" className="hover:text-white transition-colors"> | |
| Home | |
| </Link> | |
| <a href="#features" className="hover:text-white transition-colors"> | |
| Features | |
| </a> | |
| <a href="#pricing" className="hover:text-white transition-colors"> | |
| Pricing | |
| </a> | |
| <Link to="/login" className="hover:text-white transition-colors"> | |
| Login | |
| </Link> | |
| </> | |
| )} | |
| </nav> | |
| {/* ... */} | |
| </footer> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In Frontend/src/components/footer.tsx around lines 51 to 73, the footer
currently renders links to protected routes (/dashboard,
/dashboard/sponsorships, /dashboard/analytics, /dashboard/messages) which are
visible on public pages; update the component to conditionally render navigation
based on authentication state: import/use the existing auth context or
isAuthenticated hook, wrap the current dashboard links in a check that only
renders them when authenticated, and render an alternative set of public links
(e.g., About, Pricing, Contact, Login) when not authenticated; ensure you don't
remove the footer entirely and that no protected route links are rendered for
unauthenticated users.
| <div className="mt-10 h-px w-full bg-gradient-to-r from-transparent via-gray-700 to-transparent" /> | ||
|
|
||
| <p className="mt-6 text-xs text-gray-500"> | ||
| © {new Date().getFullYear()} InpactAI. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brand name inconsistency: "InpactAI" vs "Inpact".
The copyright shows "InpactAI" but the application consistently uses "Inpact" elsewhere (header, title, etc.). Maintain consistency with the established branding.
🔎 Proposed fix
- © {new Date().getFullYear()} InpactAI. All rights reserved.
+ © {new Date().getFullYear()} Inpact. All rights reserved.📝 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.
| © {new Date().getFullYear()} InpactAI. All rights reserved. | |
| © {new Date().getFullYear()} Inpact. All rights reserved. |
🤖 Prompt for AI Agents
In Frontend/src/components/footer.tsx around line 78, the footer uses the brand
name "InpactAI" which is inconsistent with the rest of the app using "Inpact";
update the string to "Inpact" so branding matches everywhere (e.g., change ©
{new Date().getFullYear()} InpactAI. All rights reserved. to use "Inpact"
instead), run a quick grep across the repo to confirm no other footer instances
remain with "InpactAI".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @Backend/app/db/db.py:
- Around line 12-13: The connection URL is getting literal "None" when env vars
are missing because raw_password (and similarly
raw_user/raw_host/raw_port/raw_dbname) can be None and are inserted into the
f-string; update the code around raw_password/PASSWORD and the corresponding
USER, HOST, PORT, DBNAME variables to either (a) validate required env vars
early and raise a clear exception if any are missing, or (b) normalize values
before URL building by setting PASSWORD = quote_plus(raw_password) if
raw_password else "" (and do the same for USER/HOST/PORT/DBNAME) so the f-string
does not contain the string "None"; ensure the code that constructs the
connection URL handles empty values correctly or fails fast with a descriptive
error.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Backend/app/db/db.py
🔇 Additional comments (1)
Backend/app/db/db.py (1)
6-6: Backend change appears unrelated to PR objectives.The PR objectives describe adding a global footer component and removing duplicate footers—purely frontend changes. This backend database configuration change (URL-encoding the password) seems unrelated and should likely be in a separate PR for clearer history and easier rollback if needed.
If this change is intentional and related, please update the PR description to explain the connection.
Also applies to: 12-13
| raw_password = os.getenv("password") | ||
| PASSWORD = quote_plus(raw_password) if raw_password else None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None values become literal string "None" in the connection URL.
When raw_password is None, PASSWORD is set to Python's None object. When interpolated into the f-string on line 19, this becomes the literal string "None", producing an invalid URL like:
postgresql+asyncpg://user:None@host:5432/dbname
The same issue applies to USER, HOST, PORT, and DBNAME if any environment variable is missing.
Suggested fix: Validate required env vars or handle None gracefully
# Fetch database credentials
USER = os.getenv("user")
raw_password = os.getenv("password")
-PASSWORD = quote_plus(raw_password) if raw_password else None
+PASSWORD = quote_plus(raw_password) if raw_password else ""
HOST = os.getenv("host")
PORT = os.getenv("port")
DBNAME = os.getenv("dbname")
+
+# Validate required environment variables
+required_vars = {"user": USER, "host": HOST, "port": PORT, "dbname": DBNAME}
+missing = [name for name, val in required_vars.items() if not val]
+if missing:
+ raise ValueError(f"Missing required environment variables: {', '.join(missing)}")Also applies to: 19-19
🤖 Prompt for AI Agents
In @Backend/app/db/db.py around lines 12 - 13, The connection URL is getting
literal "None" when env vars are missing because raw_password (and similarly
raw_user/raw_host/raw_port/raw_dbname) can be None and are inserted into the
f-string; update the code around raw_password/PASSWORD and the corresponding
USER, HOST, PORT, DBNAME variables to either (a) validate required env vars
early and raise a clear exception if any are missing, or (b) normalize values
before URL building by setting PASSWORD = quote_plus(raw_password) if
raw_password else "" (and do the same for USER/HOST/PORT/DBNAME) so the f-string
does not contain the string "None"; ensure the code that constructs the
connection URL handles empty values correctly or fails fast with a descriptive
error.
Closes #265
📝 Description
This pull request introduces a global footer component that is rendered consistently across the entire application. Previously, footer implementations were duplicated within individual pages, leading to redundancy and maintenance overhead. This PR centralizes the footer logic into a single reusable component and applies it at the app layout level.
The footer also includes a smooth scroll-based animation that triggers every time it enters the viewport, improving the overall user experience.
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
✅ Checklist
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.