Skip to content

feat: add reusable public footer (#71)#77

Open
mittalsonal wants to merge 2 commits into
suresh1319:masterfrom
mittalsonal:add-responsive-footer-71
Open

feat: add reusable public footer (#71)#77
mittalsonal wants to merge 2 commits into
suresh1319:masterfrom
mittalsonal:add-responsive-footer-71

Conversation

@mittalsonal
Copy link
Copy Markdown
Contributor

@mittalsonal mittalsonal commented May 19, 2026

Summary

  • extract the public-facing footer into a reusable SiteFooter component
  • add the footer to the landing, join, and not-found pages for consistent navigation and branding
  • update the app smoke test to verify the current landing page and footer navigation

Fixes #71

Verification

  • npm.cmd test -- --runTestsByPath src/App.test.js --runInBand --watchAll=false

Notes

  • this PR stays scoped to the public pages and does not change editor-room behavior

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor


Confidence Score: 5/5 - Safe to Merge

Safe to merge — this PR introduces a reusable public footer component with no identified issues across all six changed files. The implementation appears clean, with no logic bugs, security concerns, or runtime risks flagged by the review. The component achieves its goal of providing a shared footer for public-facing pages in a straightforward manner.

Key Findings:

  • All 6 changed files were fully reviewed and no issues of any severity were identified, including no logic errors, missing validations, or security concerns.
  • No pre-existing unresolved comments were found in this PR, meaning the review slate is entirely clean.
  • The PR is scoped appropriately as a UI/component addition (public footer), which carries inherently low risk compared to backend logic or authentication changes.
  • Zero new review comments were generated, which is consistent with a well-structured, self-contained feature addition.

@mittalsonal mittalsonal force-pushed the add-responsive-footer-71 branch from eab65e9 to 0b07c50 Compare May 19, 2026 17:20
@mittalsonal
Copy link
Copy Markdown
Contributor Author

Rebased onto the latest master and resolved the Home.js / App.test.js conflicts by preserving the newer join-page navigation flow while keeping the shared footer in place. I reran
pm.cmd test -- --runTestsByPath src/App.test.js --runInBand --watchAll=false after the rebase and it passed.

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor

entelligence-ai-pr-reviews Bot commented May 19, 2026

EntelligenceAI PR Summary

Introduces conditional rendering of the 'Join Room' footer link via a new showJoinLink prop on SiteFooter, preventing self-referential navigation on the Join Room page.

  • Added showJoinLink boolean prop (defaults to true) in src/components/SiteFooter.js with conditional rendering logic
  • Set showJoinLink={false} on SiteFooter in src/pages/Home.js to suppress the link
  • Added test in src/pages/Home.test.js validating footer link absence on the /join route while confirming presence of 'Home' and 'GitHub' links

Confidence Score: 5/5 - Safe to Merge

Safe to merge — this PR cleanly introduces a showJoinLink boolean prop to SiteFooter with a sensible default of true, ensuring no breaking changes to existing consumers while solving the self-referential navigation problem on the Join Room page. The implementation in src/components/SiteFooter.js and its usage in src/pages/Home.js is minimal and focused, and the accompanying test in src/pages/Home.test.js validates both the absence of the footer link on the /join route and the presence of other expected footer content. No review comments were generated and heuristic analysis found zero issues across all three changed files.

Key Findings:

  • showJoinLink prop defaults to true in SiteFooter.js, meaning all existing call sites that don't pass the prop retain their current behavior with no regression risk.
  • The test in src/pages/Home.test.js correctly covers the conditional rendering logic by asserting both the negative case (link absent on /join) and confirming other footer elements remain present, providing meaningful coverage rather than trivial snapshot testing.
  • The change is appropriately scoped — only Home.js passes showJoinLink={false}, and the conditional in SiteFooter.js is a straightforward boolean guard with no side effects or complex logic paths.
Files requiring special attention
  • src/components/SiteFooter.js
  • src/pages/Home.js
  • src/pages/Home.test.js

Comment thread src/components/SiteFooter.js Outdated
@mittalsonal
Copy link
Copy Markdown
Contributor Author

Addressed the remaining footer UX nit from the automated review. The shared footer now hides the self-referential Join Room link on the /join page while keeping it on the other public pages, and I added a focused regression test for that route behavior. Verification rerun:
pm.cmd test -- --runTestsByPath src/App.test.js src/pages/Home.test.js --runInBand --watchAll=false.

@suresh1319
Copy link
Copy Markdown
Owner

@mittalsonal please mention the Issue

@mittalsonal
Copy link
Copy Markdown
Contributor Author

Added the issue reference to the PR body as requested (Fixes #71). The latest verification for this branch remains
pm.cmd test -- --runTestsByPath src/App.test.js --runInBand --watchAll=false.

@mittalsonal
Copy link
Copy Markdown
Contributor Author

Quick maintainer summary: this branch is still clean, the requested issue reference is already in the PR body (Fixes #71), and the latest verification remains:\n- npm.cmd test -- --runTestsByPath src/App.test.js --runInBand --watchAll=false

@mittalsonal
Copy link
Copy Markdown
Contributor Author

Quick maintainer summary on the current state of this branch: it is still clean, the requested issue reference is already in the PR body (Fixes #71), and the latest verification remains
pm.cmd test -- --runTestsByPath src/App.test.js --runInBand --watchAll=false.\n\nSince #80 has already merged, this is one of the remaining low-risk public-facing Collab PRs in the queue if you are clearing merge-ready items.

@mittalsonal
Copy link
Copy Markdown
Contributor Author

Quick maintainer note: this branch is still clean, the requested issue reference is already in the PR body, and the remaining check is green. If this PR is being counted in the GSSoC queue, could you please add the relevant GSSoC/difficulty labels on the PR as well so the scoring metadata is present when it gets merged?

@mittalsonal
Copy link
Copy Markdown
Contributor Author

Quick status update: this PR is still cleanly mergeable and the existing GitGuardian check is passing. Ready for review/merge when maintainers get a chance.

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.

Add Responsive Footer Section to Improve Navigation and Branding

2 participants