-
Notifications
You must be signed in to change notification settings - Fork 106
fix : Correct GitHub button link on landing page #174
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
WalkthroughThe GitHub button component was refactored to use a semantic anchor element instead of a button, with the correct repository link and external navigation attributes. The text label was updated for consistency, and accessibility attributes were added. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/pages/HomePage.tsx (1)
1-1095: No GitHub button link fix found in this file.The PR description states this PR fixes the GitHub button link on the landing page (converting
<button>to<a>and adding the correct repository URL), but this file contains only formatting changes and text adjustments. There are no changes related to:
- Converting a GitHub button from
<button>to<a>element- Adding or updating a GitHub repository link
- Fixing any broken navigation
Please verify:
- Is the GitHub button in a different file not included in this PR?
- Are the actual GitHub button changes missing from this PR?
- Should this PR contain different files?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Backend/.env-example(1 hunks)Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx(1 hunks)Frontend/src/pages/HomePage.tsx(34 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Saahi30
Repo: AOSSIE-Org/InPactAI PR: 98
File: Frontend/src/pages/CollaborationDetails.tsx:234-234
Timestamp: 2025-07-14T11:45:01.864Z
Learning: In frontend-only PRs, hardcoded URLs (like dummy PDF URLs for contracts) and mock data are acceptable and intentional placeholders for UI development, to be replaced during backend integration phase.
Learnt from: Saahi30
Repo: AOSSIE-Org/InPactAI PR: 98
File: Frontend/src/pages/CollaborationDetails.tsx:519-521
Timestamp: 2025-07-12T20:28:05.017Z
Learning: In frontend-only commits that use mock data, hardcoded progress values and similar placeholder data are acceptable and intentional. These values serve as placeholders for UI development and will be replaced with dynamic calculations once backend integration occurs.
Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx
Outdated
Show resolved
Hide resolved
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx(1 hunks)Frontend/src/components/collaboration-hub/ViewProfileModal.tsx(1 hunks)
🔇 Additional comments (2)
Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx (2)
124-124: Fix correctly addresses previous review, but unrelated to PR objective.This change correctly fixes the critical bug identified in the previous review by replacing the undefined
setOpen(false)withonClose(). The control flow now properly closes the parent modal before opening the profile modal.However, this change is unrelated to the stated PR objective of fixing the GitHub button link on the landing page (Issue #172).
1-292: The landing page file exists but does not contain the claimed GitHub button changes.The landing page file
Frontend/src/pages/HomePage.tsxis present in the codebase, but inspection reveals no GitHub button element or GitHub repository link anywhere in the file. The footer navigation contains links to Dashboard, Opportunities, Analytics, and Messages only. If the PR description claims a GitHub button was converted from<button>to<a>, this change is not reflected in the current HomePage.tsx code.Either the GitHub button implementation is missing from this PR, or the PR description's claim about landing page changes may be inaccurate.
| <div className="fixed inset-0 z-[9999] flex items-center justify-center bg-black/40 backdrop-blur-sm" onClick={onClose}> | ||
| <div className="bg-white rounded-2xl shadow-xl w-full max-w-md p-6 relative flex flex-col" onClick={(e) => e.stopPropagation()}> |
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.
Critical: Changes unrelated to PR objective.
This PR claims to fix the GitHub button link on the landing page (Issue #172), but these files are collaboration hub modal components, not the landing page. The actual landing page file with the GitHub button is not included in this PR.
Additionally, the z-index value of 9999 is extremely high and may cause stacking context conflicts with other UI elements. Consider using a more reasonable value (e.g., 50 or 100) unless there's a specific requirement for such a high z-index.
The click-outside-to-close implementation itself is correct (backdrop onClick={onClose} with inner stopPropagation), but belongs in a separate PR focused on modal UX improvements.
Apply this diff to use a more reasonable z-index:
- <div className="fixed inset-0 z-[9999] flex items-center justify-center bg-black/40 backdrop-blur-sm" onClick={onClose}>
+ <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/40 backdrop-blur-sm" onClick={onClose}>📝 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.
| <div className="fixed inset-0 z-[9999] flex items-center justify-center bg-black/40 backdrop-blur-sm" onClick={onClose}> | |
| <div className="bg-white rounded-2xl shadow-xl w-full max-w-md p-6 relative flex flex-col" onClick={(e) => e.stopPropagation()}> | |
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/40 backdrop-blur-sm" onClick={onClose}> | |
| <div className="bg-white rounded-2xl shadow-xl w-full max-w-md p-6 relative flex flex-col" onClick={(e) => e.stopPropagation()}> |
🤖 Prompt for AI Agents
In Frontend/src/components/collaboration-hub/ViewProfileModal.tsx around lines
26 to 27, this PR has unrelated modal UX changes (backdrop click-to-close and a
very high z-index of 9999) that do not address the GitHub button issue on the
landing page; revert or remove these modal changes from this PR and move them to
a separate PR focused on modal UX, and ensure the actual landing page file
containing the GitHub button fix is included in this PR; if the modal change
must remain here, replace the z-index 9999 with a reasonable value such as 50 or
100 to avoid stacking conflicts and keep the existing backdrop onClick/inner
stopPropagation behavior as-is.
ab5b092 to
28154c0
Compare
|
@chandansgowda @heychirag Hi, This PR is now cleaned up and contains only the intended GitHub button fix . |
Related Issue
Fixes #172
Summary
Fixes Issue #172. The GitHub button on the landing page was a element without navigation logic.
This PR converts it into a proper anchor element and applies the correct GitHub repository link.
Changes
- Replaced
withTesting
Summary by CodeRabbit