feat: Add Aossie github link on navbar (#16)#17
Open
akshat3410 wants to merge 3 commits intoAOSSIE-Org:mainfrom
Open
feat: Add Aossie github link on navbar (#16)#17akshat3410 wants to merge 3 commits intoAOSSIE-Org:mainfrom
akshat3410 wants to merge 3 commits intoAOSSIE-Org:mainfrom
Conversation
github-actions
bot
added
no-issue-linked
WalkthroughAdded an OS-detection hook, changed the Landing page CTA to render OS-specific download links or a scroll-to-download button, added GitHub links to the navbar (desktop and mobile), and recorded the new hook in TypeScript build metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Home1 as Landing:Home1
participant Hook as useOS
Browser->>Home1: Mount Landing page
Home1->>Hook: call useOS()
Hook-->>Home1: returns OS ("mac"/"windows"/"linux"/"unknown")
alt OS detected
Home1-->>Browser: render download link (href -> OS-specific URL)
Browser->>Browser: user clicks link -> opens new tab to href
else OS unknown
Home1-->>Browser: render "Download" button (scroll)
Browser->>Home1: user clicks -> scrollToDownloads()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Pages/Landing` page/Navbar.tsx:
- Around line 149-160: Extract the duplicated GitHub URL string into a single
constant (e.g., GITHUB_REPO_URL) and replace the hard-coded URL in both anchor
tags in the Navbar component so both desktop and mobile links reference that
constant; add the constant near the top of Navbar.tsx (or in a shared constants
file) and update the href attributes in the anchors that render the Github icon
to use GITHUB_REPO_URL.
- Around line 210-221: The "GitHub" label in the Navbar anchor is hardcoded;
update the Navbar component to use the i18n resource lookup instead (e.g.,
t('github') or a shared strings object) in place of the literal "GitHub", add
the corresponding key ("github": "GitHub") to the translation/resource files for
supported locales, and ensure the anchor still calls setIsOpen(false) and
renders the Github icon component exactly as before; also provide a sensible
fallback if the i18n key is missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4627df73-964a-48d7-babb-8e32c9317166
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/Pages/Landing page/Navbar.tsx
Comment on lines
+149
to
+160
| {/* GitHub Repository Link */} | ||
| <a | ||
| href="https://github.com/AOSSIE-Org/PictoPy" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-gray-800 dark:text-gray-300 | ||
| hover:text-black dark:hover:text-white | ||
| transition-colors duration-300" | ||
| aria-label="AOSSIE PictoPy GitHub Repository" | ||
| > | ||
| <Github className="h-5 w-5" /> | ||
| </a> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting the GitHub URL to a constant.
The GitHub repository URL https://github.com/AOSSIE-Org/PictoPy is duplicated in both the desktop (line 151) and mobile (line 211) navigation sections. Extracting it to a constant improves maintainability.
♻️ Proposed refactor
Add a constant at the top of the component or in a shared constants file:
const GITHUB_REPO_URL = "https://github.com/AOSSIE-Org/PictoPy";Then reference it in both links:
<a
- href="https://github.com/AOSSIE-Org/PictoPy"
+ href={GITHUB_REPO_URL}
target="_blank"Also applies to: 210-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Pages/Landing` page/Navbar.tsx around lines 149 - 160, Extract the
duplicated GitHub URL string into a single constant (e.g., GITHUB_REPO_URL) and
replace the hard-coded URL in both anchor tags in the Navbar component so both
desktop and mobile links reference that constant; add the constant near the top
of Navbar.tsx (or in a shared constants file) and update the href attributes in
the anchors that render the Github icon to use GITHUB_REPO_URL.
Comment on lines
+210
to
+221
| <a | ||
| href="https://github.com/AOSSIE-Org/PictoPy" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-gray-700 dark:text-gray-300 text-lg font-medium | ||
| hover:text-black dark:hover:text-white | ||
| transition duration-300 flex items-center space-x-2" | ||
| onClick={() => setIsOpen(false)} | ||
| > | ||
| <Github className="h-5 w-5" /> | ||
| <span>GitHub</span> | ||
| </a> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Externalize the user-visible string for i18n.
The "GitHub" text on line 220 is hardcoded. Per coding guidelines, user-visible strings should be externalized to resource files for internationalization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Pages/Landing` page/Navbar.tsx around lines 210 - 221, The "GitHub" label
in the Navbar anchor is hardcoded; update the Navbar component to use the i18n
resource lookup instead (e.g., t('github') or a shared strings object) in place
of the literal "GitHub", add the corresponding key ("github": "GitHub") to the
translation/resource files for supported locales, and ensure the anchor still
calls setIsOpen(false) and renders the Github icon component exactly as before;
also provide a sensible fallback if the i18n key is missing.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useOS.ts`:
- Around line 12-18: The mac detection in the useOS hook incorrectly classifies
iOS/iPadOS because mobile Apple user agents include "mac" tokens; update the mac
branch condition in the useOS hook (the logic that reads
window.navigator.userAgent and calls setOS("mac")) to exclude Apple mobile
tokens by ensuring userAgent.includes("mac") && !userAgent.includes("iphone") &&
!userAgent.includes("ipad") && !userAgent.includes("ipod") (and optionally
!userAgent.includes("ios") or other mobile markers) so iPhone/iPad visitors fall
through to the generic/mobile detection instead of the desktop mac path.
In `@src/Pages/Landing` page/Home1.tsx:
- Around line 21-25: The OS-specific CTA labels ("Download for Mac", "Download
for Windows", "Download for Linux (.deb)") and the "Looking for another OS?"
string are hard-coded in the Home1 component; extract these into i18n resource
keys (e.g. landing.cta.download_mac, landing.cta.download_windows,
landing.cta.download_linux, landing.cta.other_os) and replace the literal
strings in the switch/return objects and at the other occurrence (the "Looking
for another OS?" usage) with the i18n lookups (e.g. using t('...')). Update any
tests or snapshot text if present and ensure the href values remain unchanged.
- Around line 20-25: The download CTAs in Home1.tsx currently return dead
fragment links; update the cases for "mac", "windows", and "linux" to point to
the in-page downloads anchor by returning href: "#downloads-section" (e.g., keep
text as-is but replace href for the three cases), or wire them to the real
release URLs in src/Pages/pictopy-landing.tsx if available; locate the switch
handling those platform strings in Home1.tsx and change the href values
accordingly.
In `@tsconfig.app.tsbuildinfo`:
- Line 1: The tsconfig build metadata files (tsconfig.app.tsbuildinfo and
tsconfig.node.tsbuildinfo) are tracked and should be ignored; update .gitignore
to include these filenames, then remove them from the repository index so they
stop generating diffs (e.g., git rm --cached) while leaving local copies intact;
ensure the commit references adding "tsconfig.app.tsbuildinfo" and
"tsconfig.node.tsbuildinfo" to .gitignore and removing them from tracking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 150e6e9e-777c-4a07-ba82-0c40d49458a2
⛔ Files ignored due to path filters (1)
dist/index.htmlis excluded by!**/dist/**,!**/dist/**
📒 Files selected for processing (3)
src/Pages/Landing page/Home1.tsxsrc/hooks/useOS.tstsconfig.app.tsbuildinfo
Comment on lines
+12
to
+18
| const userAgent = window.navigator.userAgent.toLowerCase(); | ||
|
|
||
| if (userAgent.includes("mac")) { | ||
| setOS("mac"); | ||
| } else if (userAgent.includes("win")) { | ||
| setOS("windows"); | ||
| } else if (userAgent.includes("linux") && !userAgent.includes("android")) { |
There was a problem hiding this comment.
Exclude iOS/iPadOS from the macOS branch.
Apple mobile user agents include Mac OS X/Macintosh tokens, so this ordering can classify iPhone/iPad visitors as mac. That sends unsupported mobile traffic down the desktop download path instead of the generic downloads flow.
📱 Minimal fix
const userAgent = window.navigator.userAgent.toLowerCase();
+ const isAppleMobile =
+ /iphone|ipad|ipod/.test(userAgent) ||
+ (window.navigator.platform === "MacIntel" &&
+ window.navigator.maxTouchPoints > 1);
- if (userAgent.includes("mac")) {
+ if (isAppleMobile) {
+ setOS("unknown");
+ } else if (userAgent.includes("mac")) {
setOS("mac");
} else if (userAgent.includes("win")) {📝 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
| const userAgent = window.navigator.userAgent.toLowerCase(); | |
| if (userAgent.includes("mac")) { | |
| setOS("mac"); | |
| } else if (userAgent.includes("win")) { | |
| setOS("windows"); | |
| } else if (userAgent.includes("linux") && !userAgent.includes("android")) { | |
| const userAgent = window.navigator.userAgent.toLowerCase(); | |
| const isAppleMobile = | |
| /iphone|ipad|ipod/.test(userAgent) || | |
| (window.navigator.platform === "MacIntel" && | |
| window.navigator.maxTouchPoints > 1); | |
| if (isAppleMobile) { | |
| setOS("unknown"); | |
| } else if (userAgent.includes("mac")) { | |
| setOS("mac"); | |
| } else if (userAgent.includes("win")) { | |
| setOS("windows"); | |
| } else if (userAgent.includes("linux") && !userAgent.includes("android")) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useOS.ts` around lines 12 - 18, The mac detection in the useOS hook
incorrectly classifies iOS/iPadOS because mobile Apple user agents include "mac"
tokens; update the mac branch condition in the useOS hook (the logic that reads
window.navigator.userAgent and calls setOS("mac")) to exclude Apple mobile
tokens by ensuring userAgent.includes("mac") && !userAgent.includes("iphone") &&
!userAgent.includes("ipad") && !userAgent.includes("ipod") (and optionally
!userAgent.includes("ios") or other mobile markers) so iPhone/iPad visitors fall
through to the generic/mobile detection instead of the desktop mac path.
Comment on lines
+20
to
+25
| case "mac": | ||
| return { text: "Download for Mac", href: "#mac-download-placeholder" }; | ||
| case "windows": | ||
| return { text: "Download for Windows", href: "#windows-download-placeholder" }; | ||
| case "linux": | ||
| return { text: "Download for Linux (.deb)", href: "#linux-download-placeholder" }; |
There was a problem hiding this comment.
The hero download CTA now resolves to dead hashes.
#mac-download-placeholder, #windows-download-placeholder, and #linux-download-placeholder do not match any target in the provided code. src/Pages/pictopy-landing.tsx already owns the real release handling, and the only known in-page target here is #downloads-section, so desktop users now lose the primary download path.
🧭 Safe fallback until the real release URLs are wired into this component
case "mac":
- return { text: "Download for Mac", href: "#mac-download-placeholder" };
+ return { text: "Download for Mac", href: "#downloads-section" };
case "windows":
- return { text: "Download for Windows", href: "#windows-download-placeholder" };
+ return { text: "Download for Windows", href: "#downloads-section" };
case "linux":
- return { text: "Download for Linux (.deb)", href: "#linux-download-placeholder" };
+ return { text: "Download for Linux (.deb)", href: "#downloads-section" };📝 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
| case "mac": | |
| return { text: "Download for Mac", href: "#mac-download-placeholder" }; | |
| case "windows": | |
| return { text: "Download for Windows", href: "#windows-download-placeholder" }; | |
| case "linux": | |
| return { text: "Download for Linux (.deb)", href: "#linux-download-placeholder" }; | |
| case "mac": | |
| return { text: "Download for Mac", href: "#downloads-section" }; | |
| case "windows": | |
| return { text: "Download for Windows", href: "#downloads-section" }; | |
| case "linux": | |
| return { text: "Download for Linux (.deb)", href: "#downloads-section" }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Pages/Landing` page/Home1.tsx around lines 20 - 25, The download CTAs in
Home1.tsx currently return dead fragment links; update the cases for "mac",
"windows", and "linux" to point to the in-page downloads anchor by returning
href: "#downloads-section" (e.g., keep text as-is but replace href for the three
cases), or wire them to the real release URLs in src/Pages/pictopy-landing.tsx
if available; locate the switch handling those platform strings in Home1.tsx and
change the href values accordingly.
Comment on lines
+21
to
+25
| return { text: "Download for Mac", href: "#mac-download-placeholder" }; | ||
| case "windows": | ||
| return { text: "Download for Windows", href: "#windows-download-placeholder" }; | ||
| case "linux": | ||
| return { text: "Download for Linux (.deb)", href: "#linux-download-placeholder" }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Externalize the new CTA copy.
The OS-specific labels and Looking for another OS? are hard-coded in the component. Please move them into the i18n resources instead of embedding them here.
As per coding guidelines, "Internationalization: User-visible strings should be externalized to resource files (i18n)".
Also applies to: 119-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Pages/Landing` page/Home1.tsx around lines 21 - 25, The OS-specific CTA
labels ("Download for Mac", "Download for Windows", "Download for Linux (.deb)")
and the "Looking for another OS?" string are hard-coded in the Home1 component;
extract these into i18n resource keys (e.g. landing.cta.download_mac,
landing.cta.download_windows, landing.cta.download_linux, landing.cta.other_os)
and replace the literal strings in the switch/return objects and at the other
occurrence (the "Looking for another OS?" usage) with the i18n lookups (e.g.
using t('...')). Update any tests or snapshot text if present and ensure the
href values remain unchanged.
| @@ -1 +1 @@ | |||
| {"root":["./src/app.tsx","./src/main.tsx","./src/vite-env.d.ts","./src/pages/pictopy-landing.tsx","./src/pages/demo/marqu.tsx","./src/pages/faqpage/faq.tsx","./src/pages/footer/footer.tsx","./src/pages/howitworks/howitworks.tsx","./src/pages/landing page/home1.tsx","./src/pages/landing page/navbar.tsx","./src/components/ui/bouncy card features.tsx","./src/components/ui/features.tsx","./src/components/ui/scrollprogress.tsx","./src/components/ui/smoothscroll.tsx","./src/components/ui/button.tsx","./src/components/ui/card.tsx","./src/components/ui/input.tsx","./src/components/ui/label.tsx","./src/context/theme-provider.tsx","./src/lib/utils.ts"],"version":"5.7.3"} No newline at end of file | |||
| {"root":["./src/app.tsx","./src/main.tsx","./src/vite-env.d.ts","./src/pages/pictopy-landing.tsx","./src/pages/demo/marqu.tsx","./src/pages/faqpage/faq.tsx","./src/pages/footer/footer.tsx","./src/pages/howitworks/howitworks.tsx","./src/pages/landing page/home1.tsx","./src/pages/landing page/navbar.tsx","./src/components/ui/bouncy card features.tsx","./src/components/ui/features.tsx","./src/components/ui/scrollprogress.tsx","./src/components/ui/smoothscroll.tsx","./src/components/ui/button.tsx","./src/components/ui/card.tsx","./src/components/ui/input.tsx","./src/components/ui/label.tsx","./src/context/theme-provider.tsx","./src/hooks/useos.ts","./src/lib/utils.ts"],"version":"5.7.3"} No newline at end of file | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "Tracked .tsbuildinfo files:"
git ls-files | rg '(^|/)[^/]+\.tsbuildinfo$' || true
echo
echo ".gitignore rules mentioning tsbuildinfo:"
rg -n --hidden --glob '.gitignore' 'tsbuildinfo'Repository: AOSSIE-Org/PictoPy-Website
Length of output: 190
Add .tsbuildinfo files to .gitignore.
Generated TypeScript build metadata files (tsconfig.app.tsbuildinfo and tsconfig.node.tsbuildinfo) are currently tracked. These create noisy, environment-specific diffs across machines and should not be versioned. Add them to .gitignore.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.app.tsbuildinfo` at line 1, The tsconfig build metadata files
(tsconfig.app.tsbuildinfo and tsconfig.node.tsbuildinfo) are tracked and should
be ignored; update .gitignore to include these filenames, then remove them from
the repository index so they stop generating diffs (e.g., git rm --cached) while
leaving local copies intact; ensure the commit references adding
"tsconfig.app.tsbuildinfo" and "tsconfig.node.tsbuildinfo" to .gitignore and
removing them from tracking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a GitHub social link to the PictoPy navigation bar, linking directly to the AOSSIE-Org/PictoPy repository.
Why add this?
Our code is open-source, and as a project under AOSSIE, it's really important that we make it as easy as possible for users and potential contributors to find our source code.
By having a visible GitHub icon directly in the navbar:
Changes Made:
lucide-react.Screenshot:
Summary by CodeRabbit