Feat/home page#65
Open
rahul-vyas-dev wants to merge 4 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughThe app now uses a shared layout shell, new reusable UI primitives, and updated build/type configuration. HomePage now fetches debounced GitHub organization suggestions, manages chips and keyboard selection, and renders composed hero and stats sections. ChangesOrgExplorer foundation and Home experience refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
no-issue-linked
Contributor
Author
|
@Zahnentferner, again, there are too many insertions and deletions because I ran Prettier formatting on the files. Also, while switching branches, some line endings changed from CRLF to LF, which caused additional formatting changes. I’ve now added a |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 18-19: The concurrency block was changed to use group:
github-pages and cancel-in-progress: true which may abort in-flight deployments
and leave the site in an inconsistent state; revert or adjust this by setting
the concurrency group back to the previous stable identifier (e.g., group:
pages) and set cancel-in-progress: false, or if you intentionally want
cancellations, add safeguards (e.g., only enable cancel-in-progress for
non-production branches) — update the concurrency settings in the deploy
workflow where the concurrency: group and cancel-in-progress values are defined.
In `@eslint.config.js`:
- Line 11: The ESLint config currently restricts linting via the files array
entry files: ["**/*.{ts,tsx}"] in eslint.config.js, so JS/JSX files are skipped;
update that files glob (the files property) to include JavaScript and JSX (for
example "**/*.{js,jsx,ts,tsx}" or a similar pattern that covers your src/**
tree) so existing src/*.js and src/*.jsx (e.g., App.jsx, hooks/useSortedData.js)
are linted by the React/React Hooks rules and JS-specific checks.
In `@package.json`:
- Line 3: The package.json "version" field was changed from 2.0.0 to 1.0.0 (a
major downgrade); either revert the package.json "version" property back to the
previous 2.0.0 value to preserve semantic versioning, or if the downgrade is
intentional, add a clear justification to the PR description and the commit
message explaining why the major version was decreased; update the "version"
property or documentation accordingly to resolve the review comment.
In `@src/components/Home/StatsSection.jsx`:
- Around line 15-17: The user-facing label in the StatsSection component
currently reads "Girth With PAT" (the <p> element with className "mt-2 text-sm
font-semibold uppercase text-white" in StatsSection.jsx) which is a
placeholder/typo; update that string to the intended copy (for example "Repos
With PAT" or the approved metric label) so the UI shows the correct
human-readable metric name.
In `@src/components/layout/Footer.tsx`:
- Around line 74-93: The footer currently renders client-side <Link> components
from footerLinks which point to /docs, /terms, /privacy, and /status but those
routes are not registered in your router (the catch-all Navigate redirects them
to "/"); update either the routing or the footer: either add corresponding Route
entries in your router (matching the paths and mapping to the new
Docs/Terms/Privacy/Status page components) or change the items in footerLinks
(or the rendering in Footer's Link map) to render external <a> anchors with
proper hrefs and target/rel attributes so they go to the real destinations;
locate the footerLinks array and the Link usage in Footer (the mapped Link
element) and modify accordingly.
In `@src/components/layout/Navbar.tsx`:
- Around line 34-41: In Navbar.tsx update the logo <img> for OrgExplorerLogo so
it no longer overflows the header and is loaded eagerly: replace the oversized
className "h-[10em]" with a height that fits the h-20 header (e.g., a Tailwind
height like "h-16" or "h-8"/"h-10" depending on desired padding) and change
loading="lazy" to loading="eager" (or remove the lazy attribute) on the <img>
element to ensure it's loaded above the fold.
- Line 23: The Tailwind variant used in the Navbar component's className is
using the incorrect "supports-backdrop-filter:…" form so the bg-black/80 rule
isn't generated; update the className on the Navbar (the JSX element that
currently contains className="... supports-backdrop-filter:bg-black/80 ...") to
use the Tailwind v4 feature-query bracketed variant by replacing the
supports-backdrop-filter token with the supports-[backdrop-filter] variant so
the backdrop-filter conditional background utility is emitted.
- Around line 71-75: The Button in Navbar.tsx is nesting a Link (anchor) inside
a real <button>, causing invalid interactive nesting; update the markup to
render the Link as the Button by using the Button's asChild prop (Radix Slot) so
the Link becomes the rendered element—replace the current Button/Link nesting
around FiSettings with a single Button with asChild and the Link
(to="/settings") as its direct child, keeping the FiSettings icon inside the
Link.
In `@src/components/RateLimitBanner.jsx`:
- Around line 32-37: In RateLimitBanner.jsx (RateLimitBanner component)
externalize all user-visible hardcoded strings — "API RATE LIMIT:", "REQUESTS
REMAINING", "Add PAT for 5,000 req/hr", and "RESETS HOURLY" — into your i18n
resource files and replace the literals with i18n lookups (e.g.,
t('apiRateLimit'), t('requestsRemaining'), t('addPat'), t('resetsHourly')) where
those strings appear (around the JSX that renders rateLimit.remaining/limit and
the PAT conditional). Ensure you import and use the project's i18n helper (e.g.,
useTranslation or t) and provide keys in the resource JSONs for each string,
updating any other occurrences at lines referenced (the strings at the other JSX
locations) to the same i18n keys.
In `@src/components/UI.jsx`:
- Around line 213-218: The component currently hardcodes user-visible strings
("Load More" button label and "Showing {shown} of {total}" text) inside the
render (button with onClick={onLoad} and the <p> showing shown/total); extract
these into i18n resource keys (e.g. load_more and showing_count) and replace the
literals with lookups from your app's i18n API (use the existing hook or helper
your project uses), passing shown and total as interpolation params for the
second string; add the new keys to the translation resource files for supported
locales and ensure the component imports/uses the translation function before
rendering (keeping the button props like C.btn("primary") and onLoad unchanged).
In `@src/components/ui/Button.tsx`:
- Around line 83-99: The Button currently sets Comp = asChild ? Slot : "button"
but always passes disabled={disabled || loading}; update the render so the
native disabled prop is only applied when Comp is the real "button" (i.e.,
asChild is false) and when asChild is true forward a11y-compatible semantics:
set aria-disabled={disabled || loading} on the Slot and add prevention of
activation (e.g., stop onClick/keyboard activation or add pointer-events-none /
tabIndex={-1} behavior) when disabled/loading; keep existing refs, className,
buttonVariants, and other props intact and ensure click/keyboard handlers are
suppressed for the Slot case so the control is non-interactive when disabled or
loading.
In `@src/context/AppContext.jsx`:
- Around line 48-50: Replace hardcoded user-visible error strings in
AppContext.jsx (the thrown Error message "No valid organizations found. Check
the names and try again." and the other messages around lines 89-93) with
references to your i18n resource keys and use the project's i18n helper (e.g.,
t('...') or i18n.translate) so messages are loaded from resource files;
create/choose appropriate keys like "errors.no_valid_organizations" and
"errors.github_rate_limit" in the locale files and update the Error construction
and any user-facing logging to use those keys instead of literal English text.
- Line 14: The PAT is being persisted insecurely via localStorage in the
AppContext component (state variables pat and setPat); replace localStorage
reads/writes with a secure storage mechanism—e.g., use expo-secure-store's
SecureStore.getItemAsync/SecureStore.setItemAsync (or migrate to HTTP-only
cookies handled by your backend) and update the initialization of pat and any
setPat calls to asynchronously load/save the token through SecureStore (or the
cookie API), removing direct localStorage usage to mitigate XSS token theft.
In `@src/pages/HomePage.jsx`:
- Around line 34-36: The fetch URL is interpolating debouncedQuery raw which
breaks on spaces/special chars; in HomePage.jsx before calling fetch (the
expression that builds
`https://api.github.com/search/users?q=${debouncedQuery}+type:org`) encode the
search portion with encodeURIComponent (for example encode the combined string
`${debouncedQuery} type:org` or separately encode debouncedQuery and append
`+type:org`) and use that encoded value in the q= param so the request is
well-formed; update the code that constructs the URL used by the fetch call to
reference the encoded query.
- Line 26: The JSON.parse call for const recent =
JSON.parse(localStorage.getItem("oe_recent") || "[]"); can throw on malformed
data; wrap this in a safe parse (e.g., try/catch or a small helper like
parseJsonSafe) so that on any parse error you return an empty array fallback and
avoid throwing during render; update the reference where recent is defined in
HomePage.jsx to use the safe parser (or fallback) instead of calling JSON.parse
directly.
- Around line 8-10: The imports in HomePage.jsx reference
"`@/components/home/`..." but the actual component folder is capitalized "Home",
causing failures on case-sensitive filesystems; update the import paths for
HeroSection, StatsSection, and OrgExplorerFeatures in src/pages/HomePage.jsx to
use "`@/components/Home/`..." (matching the directory name and each component
file) so the import specifiers exactly match the filesystem casing.
In `@src/styles/global.css`:
- Around line 65-69: Rename any camelCase keyframe names to kebab-case (e.g.,
change the `@keyframes` definition and all references from "fadeUp" to "fade-up");
update the `@keyframes` block named fadeUp and any CSS rules or JS that set
animation-name or animation: fadeUp to use "fade-up" instead, and ensure other
keyframe names (like "spin") already follow kebab-case or are renamed
consistently; search for the identifiers fadeUp and any usages in stylesheets,
components, or inline styles and update them to the new kebab-case name so
definitions and references match.
🪄 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: f2eeba0a-f2ad-46b5-b787-3743e976d5b4
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/logos/org-explorer-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (51)
.coderabbit.yaml.gitattributes.github/ISSUE_TEMPLATE/good_first_issue.yml.github/PULL_REQUEST_TEMPLATE.md.github/release-drafter.yml.github/workflows/deploy.yml.github/workflows/sync-pr-labels.yml.github/workflows/version-release.yml.pre-commit-config.yaml.prettierignore.prettierrcCONTRIBUTING.mdREADME.mdeslint.config.jspackage.jsonsrc/App.jsxsrc/components/Home/HeroSection.jsxsrc/components/Home/OrgExplorerFeatures.jsxsrc/components/Home/OrgSearchBox.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/SearchSuggestions.jsxsrc/components/Home/StatsSection.jsxsrc/components/Navbar.jsxsrc/components/RateLimitBanner.jsxsrc/components/UI.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Layout.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/context/AppContext.jsxsrc/hooks/useDebounce.tssrc/hooks/useSortedData.jssrc/lib/utils.tssrc/main.jsxsrc/pages/AnalyticsPage.jsxsrc/pages/ContributorsPage.jsxsrc/pages/GovernancePage.jsxsrc/pages/HomePage.jsxsrc/pages/NetworkPage.jsxsrc/pages/OverviewPage.jsxsrc/pages/RepositoriesPage.jsxsrc/pages/SettingsPage.jsxsrc/services/analytics.jssrc/services/github.jssrc/styles/global.csstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.jsvite.config.ts
💤 Files with no reviewable changes (2)
- vite.config.js
- src/components/Navbar.jsx
github-actions
Bot
added
size/XL
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 (4)
src/components/Home/OrgSearchBox.jsx (4)
1-7:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing "use client" directive for Next.js client component.
This component uses client-side features (event handlers:
onClick,onChange,onBlur,onFocus,onSubmit,onKeyDown) and must include the"use client"directive at the top of the file. As per coding guidelines, Next.js requires this directive for any component using client-side features.⚛️ Add "use client" directive
+"use client"; + import { FiSearch, FiX } from "react-icons/fi"; import { BsArrowRight } from "react-icons/bs";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 1 - 7, Add the Next.js client directive to the top of the OrgSearchBox component file so it runs as a client component; specifically insert "use client" as the very first line of src/components/Home/OrgSearchBox.jsx (the component that renders OrgSearchBox and imports/uses SearchSuggestions and attaches onClick, onChange, onBlur, onFocus, onSubmit, onKeyDown handlers) so Next.js recognizes the event-driven behavior.
56-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential duplicate chip and setTimeout memory leak.
Two issues in the
onBlurhandler:
Duplicate chip creation risk: Line 61 adds a chip on blur. However, from the parent's
handleKeylogic (context: HomePage.jsx), pressing Enter or comma also callsaddChip(input)but does not clear the input. This means typing text → pressing Enter → blurring will attempt to add the same chip twice (unlessaddChipinternally deduplicates).setTimeout not cleaned up: Lines 57-59 schedule
setShowSuggestions(false)without cleanup. If a suggestion is clicked,handleSelectOrgnavigates away immediately, unmounting the component while the timeout is still pending. This will trigger a "Can't perform a React state update on an unmounted component" warning.🔧 Recommended fixes
Fix 1: Clear input after adding chip to prevent duplicates
In the parent component's
handleKeyfunction, clear the input after adding a chip:if ((e.key === "Enter" || e.key === ",") && input.trim()) { e.preventDefault(); addChip(input); setInput(""); // Add this line }Fix 2: Clean up setTimeout
Convert onBlur to use useEffect or store timeout ID:
+const blurTimeoutRef = useRef(null); + +// ... in return: + onBlur={() => { - setTimeout(() => { + blurTimeoutRef.current = setTimeout(() => { setShowSuggestions(false); }, 200); input.trim() && addChip(input); }} + +// Add cleanup effect: +useEffect(() => { + return () => { + if (blurTimeoutRef.current) clearTimeout(blurTimeoutRef.current); + }; +}, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 56 - 62, The onBlur handler risks creating duplicate chips and leaking a pending timeout; update the flow so addChip duplicates are avoided and the timeout is cleaned up: ensure the caller (HomePage.handleKey) clears the input (call setInput("") after addChip) so pressing Enter/comma won't leave the same text to be added again on blur, and replace the inline setTimeout in OrgSearchBox.jsx (the onBlur that calls setShowSuggestions(false)) with a cancellable timer (store the timeout id in a ref and clearTimeout in a useEffect cleanup or clear it in onFocus/onClick handlers) so handleSelectOrg navigation/unmount doesn't trigger a state update on an unmounted component.
76-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExternalize button text for internationalization.
The "Explore" button text is hardcoded. As per coding guidelines, all user-visible strings should be externalized to i18n resource files.
🌐 Example i18n implementation
- Explore + {t('search.exploreButton')} <BsArrowRight className="size-4" />As per coding guidelines, user-visible strings must be externalized for internationalization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` at line 76, In OrgSearchBox.jsx replace the hardcoded "Explore" string in the JSX button element with a localized key (e.g. t('button.explore')) by importing/using your i18n hook (such as useTranslation from react-i18next) inside the OrgSearchBox component, call t('button.explore') where the "Explore" text currently appears, and add the corresponding "button.explore" entry to your i18n resource files (JSON) for each supported locale; ensure a sensible fallback is provided if the key is missing.
8-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNew component should be TypeScript (.tsx) with explicit prop types.
The PR description states TypeScript configuration was added and code was migrated from JavaScript to TypeScript. However, this new component file uses
.jsxextension with no type annotations for the 13 props. As per coding guidelines, explicit types should be used, and type imports should be preferred.🔷 Convert to TypeScript with prop interface
Rename file to
OrgSearchBox.tsxand add prop types:+interface OrgSearchBoxProps { + input: string; + setInput: (value: string) => void; + chips: string[]; + addChip: (chip: string) => void; + removeChip: (chip: string) => void; + handleKey: (e: React.KeyboardEvent<HTMLInputElement>) => void; + handleSubmit: (e: React.FormEvent<HTMLFormElement>) => void; + showSuggestions: boolean; + setShowSuggestions: (show: boolean) => void; + setSelectedIndex: (index: number) => void; + filteredSuggestions: Array<{ id: number; login: string; avatar_url: string }>; + selectedIndex: number; + isLoading: boolean; + handleSelectOrg: (org: string) => Promise<void>; +} + -export default function OrgSearchBox({ +export default function OrgSearchBox({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 8 - 23, Rename OrgSearchBox.jsx to OrgSearchBox.tsx and add explicit TypeScript prop types by declaring an interface (e.g., OrgSearchBoxProps) that lists each prop (input, setInput, chips, addChip, removeChip, handleKey, handleSubmit, showSuggestions, setShowSuggestions, setSelectedIndex, filteredSuggestions, selectedIndex, isLoading, handleSelectOrg) with appropriate types (string, React.Dispatch/SetStateAction, arrays, functions with proper parameter/return types, boolean, number, etc.), import necessary React types (e.g., FC, Dispatch, SetStateAction) and use them on the component signature (export default function OrgSearchBox(props: OrgSearchBoxProps) or const OrgSearchBox: FC<OrgSearchBoxProps>), and update any implicit any callbacks/arrays (like filteredSuggestions and chips) to have concrete element types so the file compiles as TypeScript.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 53-54: Replace the hardcoded user-facing strings in the
OrgSearchBox component with i18n keys: import and use your i18n accessor (e.g.,
useTranslation().t) inside OrgSearchBox and replace the aria-label and
placeholder values with calls like t('orgSearch.ariaLabel') and
t('orgSearch.placeholder'); then add corresponding entries to the translation
resource files (for all supported locales) such as orgSearch.ariaLabel and
orgSearch.placeholder with the original English text and update any unit/story
tests that assert on these strings to use the i18n keys or translated values.
---
Outside diff comments:
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 1-7: Add the Next.js client directive to the top of the
OrgSearchBox component file so it runs as a client component; specifically
insert "use client" as the very first line of
src/components/Home/OrgSearchBox.jsx (the component that renders OrgSearchBox
and imports/uses SearchSuggestions and attaches onClick, onChange, onBlur,
onFocus, onSubmit, onKeyDown handlers) so Next.js recognizes the event-driven
behavior.
- Around line 56-62: The onBlur handler risks creating duplicate chips and
leaking a pending timeout; update the flow so addChip duplicates are avoided and
the timeout is cleaned up: ensure the caller (HomePage.handleKey) clears the
input (call setInput("") after addChip) so pressing Enter/comma won't leave the
same text to be added again on blur, and replace the inline setTimeout in
OrgSearchBox.jsx (the onBlur that calls setShowSuggestions(false)) with a
cancellable timer (store the timeout id in a ref and clearTimeout in a useEffect
cleanup or clear it in onFocus/onClick handlers) so handleSelectOrg
navigation/unmount doesn't trigger a state update on an unmounted component.
- Line 76: In OrgSearchBox.jsx replace the hardcoded "Explore" string in the JSX
button element with a localized key (e.g. t('button.explore')) by
importing/using your i18n hook (such as useTranslation from react-i18next)
inside the OrgSearchBox component, call t('button.explore') where the "Explore"
text currently appears, and add the corresponding "button.explore" entry to your
i18n resource files (JSON) for each supported locale; ensure a sensible fallback
is provided if the key is missing.
- Around line 8-23: Rename OrgSearchBox.jsx to OrgSearchBox.tsx and add explicit
TypeScript prop types by declaring an interface (e.g., OrgSearchBoxProps) that
lists each prop (input, setInput, chips, addChip, removeChip, handleKey,
handleSubmit, showSuggestions, setShowSuggestions, setSelectedIndex,
filteredSuggestions, selectedIndex, isLoading, handleSelectOrg) with appropriate
types (string, React.Dispatch/SetStateAction, arrays, functions with proper
parameter/return types, boolean, number, etc.), import necessary React types
(e.g., FC, Dispatch, SetStateAction) and use them on the component signature
(export default function OrgSearchBox(props: OrgSearchBoxProps) or const
OrgSearchBox: FC<OrgSearchBoxProps>), and update any implicit any
callbacks/arrays (like filteredSuggestions and chips) to have concrete element
types so the file compiles as TypeScript.
🪄 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: a0de8331-50e1-4eb3-9750-a3cfe3256042
📒 Files selected for processing (1)
src/components/Home/OrgSearchBox.jsx
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
rahul-vyas-dev
force-pushed
the
feat/home-page
branch
from
a68113a to
4b72098
Compare
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (4)
src/components/layout/Navbar.tsx (3)
71-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix invalid interactive nesting.
Nesting a
Link(anchor) inside aButton(button element) creates invalid HTML. TheButtoncomponent supports theasChildprop (Radix Slot) to render the Link as the underlying element.🛠️ Proposed fix
- <Button variant="ghost" size="icon" aria-label="Settings"> - <Link to={"/settings"}> + <Button asChild variant="ghost" size="icon" aria-label="Settings"> + <Link to="/settings"> <FiSettings /> </Link> </Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 71 - 75, In Navbar.tsx fix the invalid interactive nesting by rendering the navigation Link as the Button's underlying element using the Button's asChild prop: replace the current Button wrapping a Link (Button > Link > FiSettings) with Button asChild so the Link becomes the actual rendered element and keep FiSettings as the child; update the JSX around Button, Link, and FiSettings accordingly to remove the nested anchor-in-button structure while preserving props like variant="ghost", size="icon", aria-label="Settings" and the to={"/settings"} on the Link.
34-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLogo dimensions and loading strategy need adjustment.
The logo height
h-[10em](~160px) overflows theh-20(80px) header. Additionally,loading="lazy"on an above-the-fold element can delay rendering and hurt LCP performance.🛠️ Proposed fix
<img src={OrgExplorerLogo} alt="OrgExplorer" - loading="lazy" + loading="eager" decoding="async" draggable={false} - className="h-[10em]" + className="h-16 w-auto" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 34 - 41, The logo image in Navbar.tsx uses className "h-[10em]" which overflows the header height (h-20) and also uses loading="lazy" which can harm above-the-fold LCP; update the <img src={OrgExplorerLogo} ... /> element to match the header by replacing "h-[10em]" with a constrained height (e.g., "h-20" or "max-h-20" / "h-auto" with an explicit max height) and remove loading="lazy" (or set it to "eager") so the logo loads promptly; ensure you keep decoding="async" and draggable={false}, and consider adding explicit width/height attributes or use intrinsic sizing to preserve aspect ratio.
23-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the correct Tailwind v4 syntax for feature queries.
supports-backdrop-filter:bg-black/80is invalid Tailwind v4 syntax. Feature queries must use the bracketedsupports-[...]form.🛠️ Proposed fix
- className="sticky top-0 z-50 w-full border-b-2 border-zinc-800 bg-black/95 backdrop-blur supports-backdrop-filter:bg-black/80" + className="sticky top-0 z-50 w-full border-b-2 border-zinc-800 bg-black/95 backdrop-blur supports-[backdrop-filter]:bg-black/80"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` at line 23, The Tailwind feature query token in the Navbar component's className is using the invalid `supports-backdrop-filter:bg-black/80`; update the className in Navbar.tsx (the JSX element with the className string shown) to use the bracketed feature query form, e.g. replace `supports-backdrop-filter:bg-black/80` with `supports-[backdrop-filter]:bg-black/80` so it conforms to Tailwind v4 syntax.src/components/layout/Footer.tsx (1)
74-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFooter navigation links point to unregistered routes.
The footer links
/docs,/terms,/privacy, and/statusare not defined insrc/App.jsx. The catch-all route will redirect them to/, resulting in broken navigation. Either add the corresponding routes or convert these to external links.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Footer.tsx` around lines 74 - 93, Footer links defined in footerLinks are using the SPA Link component but point to routes that aren't registered in the App component's route definitions (/docs, /terms, /privacy, /status), causing the catch-all to redirect to /. Fix by either (A) adding corresponding Route entries to the App component for those paths (create Docs/Terms/Privacy/Status pages and register them in the router), or (B) change the Footer.tsx Link usage to external anchors for those items (replace Link with <a href="..." target="_blank" rel="noopener noreferrer"> or set item.external flag and render <a> for external links) so they don't depend on client routes; update the footerLinks data structure or rendering logic in Footer.tsx to reflect the chosen approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Home/HeroSection.jsx`:
- Around line 24-30: The HeroSection component currently hardcodes user-visible
strings in the h1/span and p elements (“Architect Your”, “Insights”, and the
subtitle), which must be externalized to i18n resources; replace these literals
with calls to your i18n accessors (e.g., useTranslation hook or t('key')) inside
HeroSection so the h1 (and inner span), the paragraph, and the additional
strings referenced at lines 42-43 use translation keys (e.g., hero.title.part1,
hero.title.highlight, hero.subtitle) and load those keys from the locale
resource files, updating the locale JSON/YAML accordingly and ensuring any JSX
markup (span for highlighted text) still composes the translated pieces.
In `@src/components/Home/OrgExplorerFeatures.jsx`:
- Around line 5-11: The hardcoded user-facing strings in the OrgExplorerFeatures
component (the title, desc, and bullets object fields) must be moved into i18n
resource keys and referenced via the app's translation helper (e.g.,
useTranslation()/t) instead of inline literals; create keys like
"orgExplorer.features.interactive.title",
"orgExplorer.features.interactive.desc", and
"orgExplorer.features.interactive.bullets.[0..n]" in the locale files, import
and call t(...) inside OrgExplorerFeatures.jsx to populate title/desc/bullets,
and apply the same externalization pattern to the other hardcoded blocks noted
(lines 43-49, 79-85, 106-112, 136-138, 173-184) so all user-visible strings come
from the i18n resources.
- Around line 18-25: The render uses Math.random() inside the JSX map in the
OrgExplorerFeatures component which recalculates positions on every render and
causes jitter; fix it by computing a stable array of 8 position objects
(top/left) once and reusing them in the map (e.g., in OrgExplorerFeatures use
useMemo or useState initialized once to generate positions or a seeded RNG),
then replace the inline Math.random() expressions in the map with those
precomputed position values so the dots no longer change on each render.
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 56-62: The onBlur handler currently always calls addChip(input)
which creates unwanted chips during pointer-driven suggestion selection; update
the onBlur in the OrgSearchBox component to only hide suggestions (call
setShowSuggestions(false)) and remove the unconditional addChip call, and
instead add chips from the explicit user actions: handle Enter in the input's
onKeyDown to call addChip(trimmedInput) and call addChip from your suggestion
click handler; alternatively, if you must keep adding on blur, guard it by
inspecting the blur event's relatedTarget/nextActiveElement to ensure focus did
not move to a suggestion before calling addChip(input).
- Around line 40-44: Replace the clickable-only FiX icon with a proper
accessible button element so keyboard users can remove chips: wrap or replace
the FiX element with a <button> (or a Button component) that is focusable and
has type="button", an aria-label like "Remove chip" and uses the existing
onClick handler removeChip(c); ensure the button preserves the visual styling
(className "cursor-pointer opacity-70" or equivalent) and that any
stopPropagation/keyboard handling (Enter/Space) delegates to removeChip to
support activation via keyboard while keeping FiX as the visual child inside the
button.
In `@src/components/Home/QuickAccess.jsx`:
- Around line 6-8: The string "Quick Explore Access" in the JSX paragraph should
be pulled from i18n resources instead of a hard-coded literal; update the JSX in
the QuickAccess component (the <p className="text-xs uppercase tracking-[0.35em]
text-zinc-600"> element) to use the app's translation helper (e.g.,
t('quickAccess.heading') or intl.formatMessage({ id: 'quickAccess.heading' }))
and add the corresponding resource key "quickAccess.heading" with the translated
text to your i18n resource files; ensure you import or access the translation
function used across the project within QuickAccess.jsx.
In `@src/components/Home/RecentSearches.jsx`:
- Around line 10-12: The "Recent Searches" string in the RecentSearches.jsx
component is hardcoded; update the component to fetch this label from i18n
resources instead. Import and use your translation hook (e.g., useTranslation
from react-i18next) inside the RecentSearches component and replace the literal
string inside the span (the element with className "text-xs font-semibold
uppercase tracking-[0.3em] text-zinc-500") with the translated key (e.g.,
t('recentSearches') or the agreed resource key), and add the corresponding entry
to the translation resource files.
In `@src/components/Home/SearchSuggestions.jsx`:
- Around line 18-20: In SearchSuggestions.jsx replace the hardcoded user-visible
strings (e.g. the "Searching organizations..." div and the other two
suggestion/empty/loading labels in the same component) with i18n lookups: import
and call your translation helper (e.g. const { t } = useTranslation() from
react-i18next or the project's i18n utility), change the JSX to use
t('search.suggestions.searchingOrganizations') / t('search.suggestions.empty') /
t('search.suggestions.loading') (or equivalent keys), and add those keys and
translations to the locale resource files; ensure the component still renders
the same structure and fallbacks are provided if translation keys are missing.
In `@src/components/Home/StatsSection.jsx`:
- Around line 11-13: In the StatsSection.jsx component, replace all hardcoded
metric headings/descriptors (e.g., "Resource Load" and the other user-facing
strings inside the <p> elements in this component) with i18n keys and use your
app's translation helper (for example import and call the useTranslation hook/t
function) to render them (e.g., t('stats.resourceLoad')). Add corresponding keys
to the i18n resource files (e.g., stats.resourceLoad, stats.<otherKeys>) and
update the JSX in StatsSection (the <p> elements and any other metric labels) to
call t(...) instead of literal strings so all user-visible strings are
externalized.
In `@src/components/layout/Navbar.tsx`:
- Around line 95-135: The mobile nav is being mounted/unmounted with the
`{mobileMenuOpen && (...)}` conditional so its CSS transitions never run;
instead always render the <nav id="mobile-navigation"> and toggle its visibility
via classes driven by `mobileMenuOpen` (the same classnames currently used:
"max-h-96 opacity-100 translate-y-0" vs "max-h-0 opacity-0 -translate-y-2
pointer-events-none"), and also set an appropriate accessibility attribute
(e.g., aria-hidden={ !mobileMenuOpen }) so the element remains in the DOM for
the transitions inside the `nav` and the child list items rendered from
`navItems.map(...)` to animate correctly.
In `@src/components/ui/Button.tsx`:
- Around line 70-99: The button renders native <button> when asChild is false
and currently has no default type, causing accidental form submits; update the
render so that when Comp is the native "button" you pass a safe default type
that can still be overridden by callers (e.g., type={props.type ?? "button"} or
conditionally add { type: props.type ?? "button" } only when asChild is false)
and ensure this type prop is placed before spreading ...props so an explicit
props.type can override it; target the component rendering logic around Comp,
asChild, and the props spread in the Button component.
In `@src/pages/HomePage.jsx`:
- Line 12: quickExploreItems currently contains hardcoded user-visible labels;
move these labels into your i18n resource files (e.g., add keys like
quickExplore.aossieOrg, quickExplore.vercel, quickExplore.facebook,
quickExplore.microsoft) and replace the array of literal strings in HomePage.jsx
with an array of those keys or identifiers; then resolve them at render time
using the app's translation utility (e.g., useTranslation()/t or i18n.t) where
quickExploreItems is used so the UI displays translated values instead of
hardcoded strings.
- Around line 78-80: explore(orgs) currently swallows errors and sets AppContext
error state, so HomePage.jsx must not unconditionally call
navigate("/overview"); change the flow to only navigate when exploration
succeeded — either make explore (in AppContext.jsx) return a boolean/result
indicator and in HomePage.jsx do "const ok = await explore(orgs); if (ok)
navigate('/overview')" or, alternatively, read the AppContext error (e.g., via
useAppContext() after await explore(orgs)) and call navigate("/overview") only
when error is null/undefined; update the explore signature or the conditional in
HomePage.jsx accordingly so failed fetches do not trigger navigation.
In `@src/styles/global.css`:
- Around line 49-52: Remove the vendor-prefixed selection rule ::-moz-selection
and keep only the standard ::selection to satisfy the stylelint rule and avoid
duplication; locate the ::-moz-selection block in src/styles/global.css and
delete that selector and its declarations (or consolidate them under ::selection
if not already present) so only the modern ::selection styling remains.
---
Duplicate comments:
In `@src/components/layout/Footer.tsx`:
- Around line 74-93: Footer links defined in footerLinks are using the SPA Link
component but point to routes that aren't registered in the App component's
route definitions (/docs, /terms, /privacy, /status), causing the catch-all to
redirect to /. Fix by either (A) adding corresponding Route entries to the App
component for those paths (create Docs/Terms/Privacy/Status pages and register
them in the router), or (B) change the Footer.tsx Link usage to external anchors
for those items (replace Link with <a href="..." target="_blank" rel="noopener
noreferrer"> or set item.external flag and render <a> for external links) so
they don't depend on client routes; update the footerLinks data structure or
rendering logic in Footer.tsx to reflect the chosen approach.
In `@src/components/layout/Navbar.tsx`:
- Around line 71-75: In Navbar.tsx fix the invalid interactive nesting by
rendering the navigation Link as the Button's underlying element using the
Button's asChild prop: replace the current Button wrapping a Link (Button > Link
> FiSettings) with Button asChild so the Link becomes the actual rendered
element and keep FiSettings as the child; update the JSX around Button, Link,
and FiSettings accordingly to remove the nested anchor-in-button structure while
preserving props like variant="ghost", size="icon", aria-label="Settings" and
the to={"/settings"} on the Link.
- Around line 34-41: The logo image in Navbar.tsx uses className "h-[10em]"
which overflows the header height (h-20) and also uses loading="lazy" which can
harm above-the-fold LCP; update the <img src={OrgExplorerLogo} ... /> element to
match the header by replacing "h-[10em]" with a constrained height (e.g., "h-20"
or "max-h-20" / "h-auto" with an explicit max height) and remove loading="lazy"
(or set it to "eager") so the logo loads promptly; ensure you keep
decoding="async" and draggable={false}, and consider adding explicit
width/height attributes or use intrinsic sizing to preserve aspect ratio.
- Line 23: The Tailwind feature query token in the Navbar component's className
is using the invalid `supports-backdrop-filter:bg-black/80`; update the
className in Navbar.tsx (the JSX element with the className string shown) to use
the bracketed feature query form, e.g. replace
`supports-backdrop-filter:bg-black/80` with
`supports-[backdrop-filter]:bg-black/80` so it conforms to Tailwind v4 syntax.
🪄 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: 25761c05-fb83-4938-be3a-a390f28cf481
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/logos/org-explorer-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
package.jsonsrc/App.jsxsrc/components/Home/HeroSection.jsxsrc/components/Home/OrgExplorerFeatures.jsxsrc/components/Home/OrgSearchBox.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/SearchSuggestions.jsxsrc/components/Home/StatsSection.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Layout.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/hooks/useDebounce.tssrc/lib/utils.tssrc/main.jsxsrc/pages/HomePage.jsxsrc/styles/global.csstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.ts
Comment on lines
+24
to
+30
| Architect Your | ||
| <span className="block text-[#FCD34D]">Insights</span> | ||
| </h1> | ||
|
|
||
| <p className="mt-8 max-w-3xl text-base leading-relaxed text-zinc-400 sm:text-lg md:text-xl"> | ||
| Unified analytics across one or many GitHub organizations. | ||
| </p> |
There was a problem hiding this comment.
Externalize hero copy strings to i18n resources.
The heading, subtitle, and helper text are hardcoded user-visible strings in this component.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)".
Also applies to: 42-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/HeroSection.jsx` around lines 24 - 30, The HeroSection
component currently hardcodes user-visible strings in the h1/span and p elements
(“Architect Your”, “Insights”, and the subtitle), which must be externalized to
i18n resources; replace these literals with calls to your i18n accessors (e.g.,
useTranslation hook or t('key')) inside HeroSection so the h1 (and inner span),
the paragraph, and the additional strings referenced at lines 42-43 use
translation keys (e.g., hero.title.part1, hero.title.highlight, hero.subtitle)
and load those keys from the locale resource files, updating the locale
JSON/YAML accordingly and ensuring any JSX markup (span for highlighted text)
still composes the translated pieces.
Source: Coding guidelines
Comment on lines
+5
to
+11
| title: "Interactive Org Visualization", | ||
| desc: "Explore repositories, teams, and contributors through a dynamic relationship graph designed for instant understanding.", | ||
| bullets: [ | ||
| "Live node graph", | ||
| "Repository mapping", | ||
| "Contributor insights", | ||
| ], |
There was a problem hiding this comment.
Externalize feature copy to i18n resources.
Feature titles/descriptions/bullets and section copy are hardcoded user-facing strings.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)".
Also applies to: 43-49, 79-85, 106-112, 136-138, 173-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgExplorerFeatures.jsx` around lines 5 - 11, The
hardcoded user-facing strings in the OrgExplorerFeatures component (the title,
desc, and bullets object fields) must be moved into i18n resource keys and
referenced via the app's translation helper (e.g., useTranslation()/t) instead
of inline literals; create keys like "orgExplorer.features.interactive.title",
"orgExplorer.features.interactive.desc", and
"orgExplorer.features.interactive.bullets.[0..n]" in the locale files, import
and call t(...) inside OrgExplorerFeatures.jsx to populate title/desc/bullets,
and apply the same externalization pattern to the other hardcoded blocks noted
(lines 43-49, 79-85, 106-112, 136-138, 173-184) so all user-visible strings come
from the i18n resources.
Source: Coding guidelines
Comment on lines
+40
to
+44
| <FiX | ||
| size={12} | ||
| className="cursor-pointer opacity-70" | ||
| onClick={() => removeChip(c)} | ||
| /> |
There was a problem hiding this comment.
Use a real button for chip removal (keyboard access).
Lines 40-44 attach removal to an icon click only. This is not keyboard-focusable and blocks accessible chip removal for non-pointer users.
♿ Suggested fix
- <FiX
- size={12}
- className="cursor-pointer opacity-70"
- onClick={() => removeChip(c)}
- />
+ <button
+ type="button"
+ onClick={() => removeChip(c)}
+ className="cursor-pointer opacity-70"
+ aria-label={`Remove ${c}`}
+ >
+ <FiX size={12} aria-hidden="true" />
+ </button>📝 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
| <FiX | |
| size={12} | |
| className="cursor-pointer opacity-70" | |
| onClick={() => removeChip(c)} | |
| /> | |
| <button | |
| type="button" | |
| onClick={() => removeChip(c)} | |
| className="cursor-pointer opacity-70" | |
| aria-label={`Remove ${c}`} | |
| > | |
| <FiX size={12} aria-hidden="true" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgSearchBox.jsx` around lines 40 - 44, Replace the
clickable-only FiX icon with a proper accessible button element so keyboard
users can remove chips: wrap or replace the FiX element with a <button> (or a
Button component) that is focusable and has type="button", an aria-label like
"Remove chip" and uses the existing onClick handler removeChip(c); ensure the
button preserves the visual styling (className "cursor-pointer opacity-70" or
equivalent) and that any stopPropagation/keyboard handling (Enter/Space)
delegates to removeChip to support activation via keyboard while keeping FiX as
the visual child inside the button.
Comment on lines
+56
to
+62
| onBlur={() => { | ||
| setTimeout(() => { | ||
| setShowSuggestions(false); | ||
| }, 200); | ||
|
|
||
| input.trim() && addChip(input); | ||
| }} |
There was a problem hiding this comment.
onBlur should not auto-add chips unconditionally.
Lines 56-62 append input on every blur, which creates unintended chips during pointer-driven selection flows and other focus transitions.
🐛 Minimal fix
onBlur={() => {
setTimeout(() => {
setShowSuggestions(false);
}, 200);
-
- input.trim() && addChip(input);
}}📝 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
| onBlur={() => { | |
| setTimeout(() => { | |
| setShowSuggestions(false); | |
| }, 200); | |
| input.trim() && addChip(input); | |
| }} | |
| onBlur={() => { | |
| setTimeout(() => { | |
| setShowSuggestions(false); | |
| }, 200); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgSearchBox.jsx` around lines 56 - 62, The onBlur
handler currently always calls addChip(input) which creates unwanted chips
during pointer-driven suggestion selection; update the onBlur in the
OrgSearchBox component to only hide suggestions (call setShowSuggestions(false))
and remove the unconditional addChip call, and instead add chips from the
explicit user actions: handle Enter in the input's onKeyDown to call
addChip(trimmedInput) and call addChip from your suggestion click handler;
alternatively, if you must keep adding on blur, guard it by inspecting the blur
event's relatedTarget/nextActiveElement to ensure focus did not move to a
suggestion before calling addChip(input).
Comment on lines
+70
to
+99
| { | ||
| className, | ||
| variant, | ||
| size, | ||
| fullWidth, | ||
| loading, | ||
| asChild = false, | ||
| disabled, | ||
| children, | ||
| ...props | ||
| }, | ||
| ref | ||
| ) => { | ||
| const Comp = asChild ? Slot : "button"; | ||
|
|
||
| return ( | ||
| <Comp | ||
| ref={ref} | ||
| className={cn( | ||
| buttonVariants({ | ||
| variant, | ||
| size, | ||
| fullWidth, | ||
| loading, | ||
| }), | ||
| className | ||
| )} | ||
| disabled={disabled || loading} | ||
| {...props} | ||
| > |
There was a problem hiding this comment.
Set a safe default type="button" for native button renders.
Without an explicit default, this component will submit enclosing forms unintentionally when used as a plain button.
♻️ Suggested fix
const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
(
{
className,
variant,
size,
fullWidth,
loading,
asChild = false,
disabled,
+ type,
children,
...props
},
ref
) => {
const Comp = asChild ? Slot : "button";
return (
<Comp
ref={ref}
@@
- disabled={disabled || loading}
+ type={asChild ? undefined : type ?? "button"}
+ disabled={disabled || loading}
{...props}
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/Button.tsx` around lines 70 - 99, The button renders native
<button> when asChild is false and currently has no default type, causing
accidental form submits; update the render so that when Comp is the native
"button" you pass a safe default type that can still be overridden by callers
(e.g., type={props.type ?? "button"} or conditionally add { type: props.type ??
"button" } only when asChild is false) and ensure this type prop is placed
before spreading ...props so an explicit props.type can override it; target the
component rendering logic around Comp, asChild, and the props spread in the
Button component.
| import StatsSection from "@/components/home/StatsSection"; | ||
| import OrgExplorerFeatures from "@/components/home/OrgExplorerFeatures"; | ||
|
|
||
| const quickExploreItems = ["AOSSIE-Org", "vercel", "facebook", "microsoft"]; |
There was a problem hiding this comment.
Externalize quick-access labels to i18n resources.
Line 12 hardcodes user-visible org labels in code. Move these to translation resources and resolve them at render time.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/HomePage.jsx` at line 12, quickExploreItems currently contains
hardcoded user-visible labels; move these labels into your i18n resource files
(e.g., add keys like quickExplore.aossieOrg, quickExplore.vercel,
quickExplore.facebook, quickExplore.microsoft) and replace the array of literal
strings in HomePage.jsx with an array of those keys or identifiers; then resolve
them at render time using the app's translation utility (e.g.,
useTranslation()/t or i18n.t) where quickExploreItems is used so the UI displays
translated values instead of hardcoded strings.
Source: Coding guidelines
Comment on lines
+49
to
+52
| ::-moz-selection { | ||
| background: rgba(252, 211, 77, 0.9); | ||
| color: #000000; | ||
| } |
There was a problem hiding this comment.
Remove the vendor-prefixed selection rule at Line 49.
::-moz-selection violates your current stylelint rule and duplicates ::selection behavior for modern browsers.
♻️ Suggested minimal fix
-::-moz-selection {
- background: rgba(252, 211, 77, 0.9);
- color: `#000000`;
-}As per coding guidelines: "**/*.css: Review the CSS code against the google css style guide and point out any mismatches."
📝 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
| ::-moz-selection { | |
| background: rgba(252, 211, 77, 0.9); | |
| color: #000000; | |
| } |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 49-49: Vendor-prefixed selector "::-moz-selection" (selector-no-vendor-prefix)
(selector-no-vendor-prefix)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles/global.css` around lines 49 - 52, Remove the vendor-prefixed
selection rule ::-moz-selection and keep only the standard ::selection to
satisfy the stylelint rule and avoid duplication; locate the ::-moz-selection
block in src/styles/global.css and delete that selector and its declarations (or
consolidate them under ::selection if not already present) so only the modern
::selection styling remains.
Sources: Coding guidelines, Linters/SAST tools
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
github-actions
Bot
added
size/XL
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
rahul-vyas-dev
force-pushed
the
feat/home-page
branch
from
2643587 to
689cdd6
Compare
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
src/styles/global.css (1)
67-70: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the vendor-prefixed selection selector.
At Line 67,
::-moz-selectionduplicates::selectionand conflicts with strict stylelint CSS rules in this PR history.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/styles/global.css` around lines 67 - 70, Remove the vendor-prefixed selection rule because it duplicates the standard selection styling and violates the CSS lint rule. Update the selection styling in global.css by keeping the shared `::selection` selector only and deleting the `::-moz-selection` block so the browser behavior remains covered without the vendor-specific duplicate.Source: Path instructions
src/components/ui/Button.tsx (2)
86-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSet a safe default
type="button"for native button renders.At Line 86, native
<button>without an explicit defaulttypecan submit parent forms unintentionally. Settype={asChild ? undefined : (props.type ?? "button")}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Button.tsx` around lines 86 - 99, The native Button render currently passes through props without guaranteeing a safe default type, which can cause unintended form submits. Update the Button component’s Comp render logic so that when it is not using asChild, it defaults the button type to "button" unless props.type is explicitly provided, while leaving asChild behavior unchanged. Use the existing Button, Comp, asChild, and props handling in Button.tsx to apply the fix in one place.
83-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate disabled semantics by render mode (
buttonvsSlot).At Line 97, forwarding
disabledthroughSlotdoes not reliably disable non-button children; it can remain interactive while appearing disabled. Apply nativedisabledonly whenasChildis false, and usearia-disabled+ activation prevention forasChildmode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Button.tsx` around lines 83 - 99, The Button component’s disabled handling in Button.tsx should be gated by render mode: when Comp is the native button, keep using the disabled prop, but when asChild renders through Slot, do not forward disabled because it may not disable the underlying element. Update the Button render logic to branch on asChild, using aria-disabled for Slot mode and adding activation prevention for interactive children, while preserving the existing disabled || loading behavior for the button path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.jsx`:
- Around line 15-41: The file has a duplicate default export and redeclaration
of App, which prevents compilation. Keep the top route-rendering component as a
separate named component such as AppContent, and have the bottom App component
remain the single default export that wraps ThemeProvider and AppProvider around
AppContent. Make sure the Routes/Route block lives inside AppContent and that
only one export default function App exists.
In `@src/components/Home/OrgExplorerFeatures.jsx`:
- Around line 29-34: Hide the decorative connector SVG in
OrgExplorerFeatures.jsx from assistive technologies by updating the SVG element
used for the background graphic to be non-announced. Add aria-hidden="true" to
the svg, and include focusable="false" if needed for browser support, since this
graphic is purely decorative and should not be exposed to screen readers.
- Around line 150-158: The Export controls in the OrgExplorerFeatures JSX are
decorative mock buttons with no action, so they should not be reachable by
keyboard. Update the mapped UI in OrgExplorerFeatures to use non-interactive
elements instead of button elements, or otherwise remove them from the tab
order, while keeping the visual styling intact and following Lighthouse/React
accessibility best practices.
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 31-68: The main search field in OrgSearchBox currently suppresses
the default browser focus ring without adding a replacement, so keyboard users
have no visible focus cue. Update the input and/or its wrapping container in
OrgSearchBox to show a clear focus state when the field receives focus, using an
accessible focus-visible style that matches the existing UI.
In `@src/components/Home/SearchSuggestions.jsx`:
- Around line 23-29: The suggestion row buttons in SearchSuggestions.jsx are
only wired through onMouseDown, so keyboard users cannot activate them with
Enter or Space. Update the existing button handler in the suggestion rendering
logic to use standard button activation (for example, onClick or a shared select
handler used by both pointer and keyboard events) and keep the current
handleSelectOrg/addChip behavior intact for the org.login value.
In `@src/components/layout/Navbar.tsx`:
- Around line 8-15: The navbar uses hardcoded user-facing strings in Navbar.tsx,
so externalize them for i18n instead of embedding labels directly. Move the nav
item labels from navItems, and any aria/text used in the Navbar render path,
into locale resources or shared translation constants, then read them through
the existing i18n mechanism in the Navbar component so all visible navigation
copy can be translated consistently.
In `@src/main.jsx`:
- Around line 11-12: The BrowserRouter in main.jsx is hardcoded with the wrong
basename, so update the router setup to match the production base path used by
Vite. In the BrowserRouter wrapper, replace the static “/” basename with a
mode-aware value using import.meta.env.PROD so production uses “/OrgExplorer/”
and local development stays at “/”. This keeps routing and generated links
consistent in both environments.
In `@vite.config.ts`:
- Line 12: The Vite alias setup uses __dirname, which breaks in ES module
contexts. Update the alias in vite.config.ts to resolve from import.meta.url
using fileURLToPath so the "@"/src path is computed safely regardless of module
system. Make the change in the config where the alias is defined, and ensure the
file imports the ES-module-safe path helper alongside the path utilities.
---
Duplicate comments:
In `@src/components/ui/Button.tsx`:
- Around line 86-99: The native Button render currently passes through props
without guaranteeing a safe default type, which can cause unintended form
submits. Update the Button component’s Comp render logic so that when it is not
using asChild, it defaults the button type to "button" unless props.type is
explicitly provided, while leaving asChild behavior unchanged. Use the existing
Button, Comp, asChild, and props handling in Button.tsx to apply the fix in one
place.
- Around line 83-99: The Button component’s disabled handling in Button.tsx
should be gated by render mode: when Comp is the native button, keep using the
disabled prop, but when asChild renders through Slot, do not forward disabled
because it may not disable the underlying element. Update the Button render
logic to branch on asChild, using aria-disabled for Slot mode and adding
activation prevention for interactive children, while preserving the existing
disabled || loading behavior for the button path.
In `@src/styles/global.css`:
- Around line 67-70: Remove the vendor-prefixed selection rule because it
duplicates the standard selection styling and violates the CSS lint rule. Update
the selection styling in global.css by keeping the shared `::selection` selector
only and deleting the `::-moz-selection` block so the browser behavior remains
covered without the vendor-specific duplicate.
🪄 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: 862dc810-4e84-4cf8-8300-5fa63f517340
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/logos/org-explorer-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
package.jsonsrc/App.jsxsrc/components/Home/HeroSection.jsxsrc/components/Home/OrgExplorerFeatures.jsxsrc/components/Home/OrgSearchBox.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/SearchSuggestions.jsxsrc/components/Home/StatsSection.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Layout.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/hooks/useDebounce.tssrc/lib/utils.tssrc/main.jsxsrc/pages/HomePage.jsxsrc/styles/global.csstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
♻️ Duplicate comments (3)
src/styles/global.css (1)
67-70: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the vendor-prefixed selection selector.
At Line 67,
::-moz-selectionduplicates::selectionand conflicts with strict stylelint CSS rules in this PR history.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/styles/global.css` around lines 67 - 70, Remove the vendor-prefixed selection rule because it duplicates the standard selection styling and violates the CSS lint rule. Update the selection styling in global.css by keeping the shared `::selection` selector only and deleting the `::-moz-selection` block so the browser behavior remains covered without the vendor-specific duplicate.Source: Path instructions
src/components/ui/Button.tsx (2)
86-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSet a safe default
type="button"for native button renders.At Line 86, native
<button>without an explicit defaulttypecan submit parent forms unintentionally. Settype={asChild ? undefined : (props.type ?? "button")}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Button.tsx` around lines 86 - 99, The native Button render currently passes through props without guaranteeing a safe default type, which can cause unintended form submits. Update the Button component’s Comp render logic so that when it is not using asChild, it defaults the button type to "button" unless props.type is explicitly provided, while leaving asChild behavior unchanged. Use the existing Button, Comp, asChild, and props handling in Button.tsx to apply the fix in one place.
83-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate disabled semantics by render mode (
buttonvsSlot).At Line 97, forwarding
disabledthroughSlotdoes not reliably disable non-button children; it can remain interactive while appearing disabled. Apply nativedisabledonly whenasChildis false, and usearia-disabled+ activation prevention forasChildmode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Button.tsx` around lines 83 - 99, The Button component’s disabled handling in Button.tsx should be gated by render mode: when Comp is the native button, keep using the disabled prop, but when asChild renders through Slot, do not forward disabled because it may not disable the underlying element. Update the Button render logic to branch on asChild, using aria-disabled for Slot mode and adding activation prevention for interactive children, while preserving the existing disabled || loading behavior for the button path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.jsx`:
- Around line 15-41: The file has a duplicate default export and redeclaration
of App, which prevents compilation. Keep the top route-rendering component as a
separate named component such as AppContent, and have the bottom App component
remain the single default export that wraps ThemeProvider and AppProvider around
AppContent. Make sure the Routes/Route block lives inside AppContent and that
only one export default function App exists.
In `@src/components/Home/OrgExplorerFeatures.jsx`:
- Around line 29-34: Hide the decorative connector SVG in
OrgExplorerFeatures.jsx from assistive technologies by updating the SVG element
used for the background graphic to be non-announced. Add aria-hidden="true" to
the svg, and include focusable="false" if needed for browser support, since this
graphic is purely decorative and should not be exposed to screen readers.
- Around line 150-158: The Export controls in the OrgExplorerFeatures JSX are
decorative mock buttons with no action, so they should not be reachable by
keyboard. Update the mapped UI in OrgExplorerFeatures to use non-interactive
elements instead of button elements, or otherwise remove them from the tab
order, while keeping the visual styling intact and following Lighthouse/React
accessibility best practices.
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 31-68: The main search field in OrgSearchBox currently suppresses
the default browser focus ring without adding a replacement, so keyboard users
have no visible focus cue. Update the input and/or its wrapping container in
OrgSearchBox to show a clear focus state when the field receives focus, using an
accessible focus-visible style that matches the existing UI.
In `@src/components/Home/SearchSuggestions.jsx`:
- Around line 23-29: The suggestion row buttons in SearchSuggestions.jsx are
only wired through onMouseDown, so keyboard users cannot activate them with
Enter or Space. Update the existing button handler in the suggestion rendering
logic to use standard button activation (for example, onClick or a shared select
handler used by both pointer and keyboard events) and keep the current
handleSelectOrg/addChip behavior intact for the org.login value.
In `@src/components/layout/Navbar.tsx`:
- Around line 8-15: The navbar uses hardcoded user-facing strings in Navbar.tsx,
so externalize them for i18n instead of embedding labels directly. Move the nav
item labels from navItems, and any aria/text used in the Navbar render path,
into locale resources or shared translation constants, then read them through
the existing i18n mechanism in the Navbar component so all visible navigation
copy can be translated consistently.
In `@src/main.jsx`:
- Around line 11-12: The BrowserRouter in main.jsx is hardcoded with the wrong
basename, so update the router setup to match the production base path used by
Vite. In the BrowserRouter wrapper, replace the static “/” basename with a
mode-aware value using import.meta.env.PROD so production uses “/OrgExplorer/”
and local development stays at “/”. This keeps routing and generated links
consistent in both environments.
In `@vite.config.ts`:
- Line 12: The Vite alias setup uses __dirname, which breaks in ES module
contexts. Update the alias in vite.config.ts to resolve from import.meta.url
using fileURLToPath so the "@"/src path is computed safely regardless of module
system. Make the change in the config where the alias is defined, and ensure the
file imports the ES-module-safe path helper alongside the path utilities.
---
Duplicate comments:
In `@src/components/ui/Button.tsx`:
- Around line 86-99: The native Button render currently passes through props
without guaranteeing a safe default type, which can cause unintended form
submits. Update the Button component’s Comp render logic so that when it is not
using asChild, it defaults the button type to "button" unless props.type is
explicitly provided, while leaving asChild behavior unchanged. Use the existing
Button, Comp, asChild, and props handling in Button.tsx to apply the fix in one
place.
- Around line 83-99: The Button component’s disabled handling in Button.tsx
should be gated by render mode: when Comp is the native button, keep using the
disabled prop, but when asChild renders through Slot, do not forward disabled
because it may not disable the underlying element. Update the Button render
logic to branch on asChild, using aria-disabled for Slot mode and adding
activation prevention for interactive children, while preserving the existing
disabled || loading behavior for the button path.
In `@src/styles/global.css`:
- Around line 67-70: Remove the vendor-prefixed selection rule because it
duplicates the standard selection styling and violates the CSS lint rule. Update
the selection styling in global.css by keeping the shared `::selection` selector
only and deleting the `::-moz-selection` block so the browser behavior remains
covered without the vendor-specific duplicate.
🪄 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: 862dc810-4e84-4cf8-8300-5fa63f517340
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/logos/org-explorer-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
package.jsonsrc/App.jsxsrc/components/Home/HeroSection.jsxsrc/components/Home/OrgExplorerFeatures.jsxsrc/components/Home/OrgSearchBox.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/SearchSuggestions.jsxsrc/components/Home/StatsSection.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Layout.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/hooks/useDebounce.tssrc/lib/utils.tssrc/main.jsxsrc/pages/HomePage.jsxsrc/styles/global.csstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.ts
🛑 Comments failed to post (8)
src/App.jsx (1)
15-41: 🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm duplicate default export in App.jsx rg -n 'export default function App\s*\(' src/App.jsxRepository: AOSSIE-Org/OrgExplorer
Length of output: 230
Resolve duplicate default export and redeclaration of
App.This file defines
export default function App()twice (Lines 15 and 33), which is a hard compile error.Rename the first component to
AppContentand export it as shown below:Suggested fix
-export default function App() { +function AppContent() { return ( <Layout> <Routes> <Route path="/" element={<HomePage />} /> <Route path="/overview" element={<OverviewPage />} /> <Route path="/repositories" element={<RepositoriesPage />} /> <Route path="/contributors" element={<ContributorsPage />} /> <Route path="/network" element={<NetworkPage />} /> <Route path="/analytics" element={<AnalyticsPage />} /> <Route path="/governance" element={<GovernancePage />} /> <Route path="/settings" element={<SettingsPage />} /> <Route path="*" element={<Navigate to="/" replace />} /> </Routes> </Layout> ) }📝 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.function AppContent() { return ( <Layout> <Routes> <Route path="/" element={<HomePage />} /> <Route path="/overview" element={<OverviewPage />} /> <Route path="/repositories" element={<RepositoriesPage />} /> <Route path="/contributors" element={<ContributorsPage />} /> <Route path="/network" element={<NetworkPage />} /> <Route path="/analytics" element={<AnalyticsPage />} /> <Route path="/governance" element={<GovernancePage />} /> <Route path="/settings" element={<SettingsPage />} /> <Route path="*" element={<Navigate to="/" replace />} /> </Routes> </Layout> ) }🧰 Tools
🪛 Biome (2.5.0)
[error] 33-41: Illegal duplicate default export declarations
(parse)
[error] 33-33: 'App' is redeclared in the same scope.
(lint/suspicious/noRedeclare)
🪛 React Doctor (0.5.8)
[error] 15-15: A module cannot have multiple default exports.
(2528)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.jsx` around lines 15 - 41, The file has a duplicate default export and redeclaration of App, which prevents compilation. Keep the top route-rendering component as a separate named component such as AppContent, and have the bottom App component remain the single default export that wraps ThemeProvider and AppProvider around AppContent. Make sure the Routes/Route block lives inside AppContent and that only one export default function App exists.Source: Linters/SAST tools
src/components/Home/OrgExplorerFeatures.jsx (2)
29-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Hide the decorative SVG from assistive tech.
This connector graphic is background decoration, but it currently has no accessible treatment. Mark it
aria-hidden="true"(andfocusable="false"if needed) unless it is meant to be announced.As per path instructions, JSX should follow Lighthouse and React accessibility best practices.
🧰 Tools
🪛 Biome (2.5.0)
[error] 29-29: Alternative text title element cannot be empty
(lint/a11y/noSvgWithoutTitle)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgExplorerFeatures.jsx` around lines 29 - 34, Hide the decorative connector SVG in OrgExplorerFeatures.jsx from assistive technologies by updating the SVG element used for the background graphic to be non-announced. Add aria-hidden="true" to the svg, and include focusable="false" if needed for browser support, since this graphic is purely decorative and should not be exposed to screen readers.Source: Path instructions
150-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not leave decorative mock buttons in the tab order.
These controls look interactive, but they have no action. Keyboard users will still land on dead buttons in the feature mockup. If this is illustrative UI, render non-interactive elements instead.
As per path instructions, JSX should follow Lighthouse and React accessibility best practices.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 154-154: A list component should have a key to prevent re-rendering
Context: .{item.toLowerCase()}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-needs-key)
[warning] 155-157: A list component should have a key to prevent re-rendering
Context:
Export
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-needs-key)
🪛 Biome (2.5.0)
[error] 156-156: Provide an explicit type prop for the button element.
(lint/a11y/useButtonType)
🪛 React Doctor (0.5.8)
[warning] 156-156: Your users can submit the form by accident because a
<button>with notypedefaults to submit.Set an explicit button
typeso plain buttons do not submit forms by accident:type="button","submit", or"reset".(button-has-type)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgExplorerFeatures.jsx` around lines 150 - 158, The Export controls in the OrgExplorerFeatures JSX are decorative mock buttons with no action, so they should not be reachable by keyboard. Update the mapped UI in OrgExplorerFeatures to use non-interactive elements instead of button elements, or otherwise remove them from the tab order, while keeping the visual styling intact and following Lighthouse/React accessibility best practices.Source: Path instructions
src/components/Home/OrgSearchBox.jsx (1)
31-68: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore a visible focus indicator on the main search field.
The input removes the browser outline, but neither the input nor its container adds a replacement focus style. Tabbing into the primary control leaves no reliable focus cue.
As per path instructions, JSX should follow Lighthouse and React accessibility best practices.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 37-37: A list component should have a key to prevent re-rendering
Context: {c}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-needs-key)
[warning] 39-43: A list component should have a key to prevent re-rendering
Context: <FiX
size={12}
className="cursor-pointer opacity-70"
onClick={() => removeChip(c)}
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 31 - 68, The main search field in OrgSearchBox currently suppresses the default browser focus ring without adding a replacement, so keyboard users have no visible focus cue. Update the input and/or its wrapping container in OrgSearchBox to show a clear focus state when the field receives focus, using an accessible focus-visible style that matches the existing UI.Source: Path instructions
src/components/Home/SearchSuggestions.jsx (1)
23-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep suggestion rows keyboard-activatable.
These rows are rendered as
<button>elements, but the selection logic only runs ononMouseDown. A focused suggestion will not activate on Enter/Space, so keyboard and pointer behavior diverge.Suggested fix
+ const selectOrg = (login) => { + handleSelectOrg(login); + addChip(login); + }; + return ( <div className="absolute left-0 top-[105%] z-50 flex max-h-96 w-full flex-col overflow-y-auto rounded-2xl border border-zinc-800 bg-zinc-950 shadow-2xl"> @@ <button key={org.id} type="button" - onMouseDown={() => { - handleSelectOrg(org.login); - addChip(org.login); - }} + onMouseDown={() => selectOrg(org.login)} + onClick={() => selectOrg(org.login)} className={cn(As per path instructions, JSX should follow Lighthouse and React accessibility best practices.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/SearchSuggestions.jsx` around lines 23 - 29, The suggestion row buttons in SearchSuggestions.jsx are only wired through onMouseDown, so keyboard users cannot activate them with Enter or Space. Update the existing button handler in the suggestion rendering logic to use standard button activation (for example, onClick or a shared select handler used by both pointer and keyboard events) and keep the current handleSelectOrg/addChip behavior intact for the org.login value.Source: Path instructions
src/components/layout/Navbar.tsx (1)
8-15: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Externalize user-facing navbar strings for i18n compliance.
Navigation labels and aria text are hardcoded in the component; move them to locale resources/constants so they can be translated consistently.
As per path instructions, "Internationalization: User-visible strings should be externalized to resource files (i18n)".
Also applies to: 31-33, 81-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 8 - 15, The navbar uses hardcoded user-facing strings in Navbar.tsx, so externalize them for i18n instead of embedding labels directly. Move the nav item labels from navItems, and any aria/text used in the Navbar render path, into locale resources or shared translation constants, then read them through the existing i18n mechanism in the Navbar component so all visible navigation copy can be translated consistently.Source: Path instructions
src/main.jsx (1)
11-12: 🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify basename/base consistency echo "src/main.jsx:" sed -n '1,40p' src/main.jsx echo echo "vite.config.ts:" sed -n '1,40p' vite.config.tsRepository: AOSSIE-Org/OrgExplorer
Length of output: 1110
🏁 Script executed:
#!/bin/bash # Verify the actual deployment path and whether import.meta.env is used or available echo "Checking for environment variable usage in src/main.jsx" grep -n "import.meta" src/main.jsx || echo "No import.meta found" echo echo "Checking vite.config.ts for base configuration" cat vite.config.tsRepository: AOSSIE-Org/OrgExplorer
Length of output: 661
Fix router basename to match production base path.
src/main.jsxhardcodesbasename="/", butvite.config.tssets the production base to"/OrgExplorer/". This mismatch causesreact-router-domto generate incorrect links and asset paths when deployed to a subdirectory in production.Update the router to use the environment mode to set the correct base:
- <BrowserRouter basename= "/"> + <BrowserRouter basename={import.meta.env.PROD ? "/OrgExplorer/" : "/"}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.jsx` around lines 11 - 12, The BrowserRouter in main.jsx is hardcoded with the wrong basename, so update the router setup to match the production base path used by Vite. In the BrowserRouter wrapper, replace the static “/” basename with a mode-aware value using import.meta.env.PROD so production uses “/OrgExplorer/” and local development stays at “/”. This keeps routing and generated links consistent in both environments.vite.config.ts (1)
12-12: 🎯 Functional Correctness | 🟠 Major
Replace
__dirnamewith ES module-safe path resolution.The use of
__dirnameinvite.config.tsis unsafe in ESM contexts where it is undefined. Update the alias at line 12 to useimport.meta.urlwithfileURLToPathfor reliable resolution:
import { fileURLToPath } from "url"; import path from "path";
@: path.resolve(fileURLToPath(import.meta.url), "./src")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vite.config.ts` at line 12, The Vite alias setup uses __dirname, which breaks in ES module contexts. Update the alias in vite.config.ts to resolve from import.meta.url using fileURLToPath so the "@"/src path is computed safely regardless of module system. Make the change in the config where the alias is defined, and ensure the file imports the ES-module-safe path helper alongside the path utilities.
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/HomePage.jsx (1)
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClamp
selectedIndexwhen the suggestion list changes.
selectedIndexsurvives query/result changes. If the list shrinks after the user has moved down the menu, Line 111 can readfilteredSuggestions[selectedIndex].loginfromundefinedand crash on Enter. Reset or clamp the index whenfilteredSuggestionschanges.🐛 Proposed fix
-import React, { useMemo, useState } from "react"; +import React, { useEffect, useMemo, useState } from "react"; @@ const filteredSuggestions = useMemo(() => { return suggestions.filter( (item) => item.login.toLowerCase() !== input.toLowerCase() ); }, [suggestions, input]); + + useEffect(() => { + setSelectedIndex((prev) => { + if (!filteredSuggestions.length) return -1; + return prev >= filteredSuggestions.length + ? filteredSuggestions.length - 1 + : prev; + }); + }, [filteredSuggestions.length]); @@ - if (e.key === "Enter" && selectedIndex >= 0) { + if ( + e.key === "Enter" && + selectedIndex >= 0 && + selectedIndex < filteredSuggestions.length + ) {Also applies to: 50-54, 108-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` at line 1, The suggestion selection state in HomePage is not being kept in sync with the filtered list, so `selectedIndex` can point past the end of `filteredSuggestions` and crash when Enter reads from `filteredSuggestions[selectedIndex]`. Update the `HomePage` logic that derives or uses `selectedIndex` and `filteredSuggestions` to clamp the index back into range or reset it whenever the suggestion list changes, especially in the handler/effect around the search state and the Enter key path. Use the existing `selectedIndex`, `filteredSuggestions`, and the input/menu navigation code to ensure the active item always references a valid suggestion.src/components/layout/Footer.tsx (1)
139-154: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the footer copy.
© 2026 OrgExplorerandBuilt for open source communitiesare still hardcoded in the component, so this footer cannot be localized cleanly. As per path instructions, "User-visible strings should be externalized to resource files (i18n)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Footer.tsx` around lines 139 - 154, The Footer component still contains hardcoded user-facing copy, so externalize the strings used in the Footer JSX instead of embedding them directly. Update Footer.tsx to read the copyright line and the “Built for open source communities” text from the project’s i18n/resource layer, using the existing footer-related identifiers in the component to locate the text nodes. Keep the component focused on rendering and ensure the localized keys are added to the appropriate resource file so the footer can be translated cleanly.Source: Path instructions
src/components/layout/Navbar.tsx (1)
33-35: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the new navbar copy.
This component hardcodes user-facing labels and ARIA text, which keeps the navigation out of the app’s localization path. Move the strings into resource files instead of embedding them in the component. As per path instructions, "User-visible strings should be externalized to resource files (i18n)."
Also applies to: 52-52, 92-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 33 - 35, The Navbar component is hardcoding user-facing copy, including the homepage link ARIA label and other visible nav text, so it bypasses localization. Move these strings out of Navbar.tsx into the app’s i18n/resource files and read them through the existing translation mechanism. Update the affected Navbar render paths and any related labels/menus in the same component so all user-visible text is sourced from localized keys instead of inline literals.Source: Path instructions
♻️ Duplicate comments (9)
src/components/Home/QuickAccess.jsx (1)
6-8: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the quick-access heading.
Quick Explore Accessis still hardcoded in the component. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/QuickAccess.jsx` around lines 6 - 8, The QuickAccess heading is still hardcoded as user-visible text, so externalize it to the i18n/resource layer instead of rendering the literal string in QuickAccess. Update the component to read the heading from the existing translation/resource system used elsewhere in the app, and keep the visible label unchanged through that localized key.Source: Path instructions
src/components/Home/RecentSearches.jsx (1)
10-12: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the section heading.
Recent Searchesis still a user-visible literal. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/RecentSearches.jsx` around lines 10 - 12, The section heading in RecentSearches is still hardcoded and should be externalized for i18n. Update the RecentSearches component to replace the literal “Recent Searches” with the appropriate localized resource lookup used elsewhere in the app, so the heading is user-visible through the resource file instead of inline text.Source: Path instructions
src/pages/HomePage.jsx (5)
25-25: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard and validate
oe_recentbefore render.A malformed value here still throws during render, and a valid-but-non-array payload will break later when
recent.map(...)runs. Parse this defensively and fall back to[]unless the result is actually an array.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` at line 25, The `HomePage` render path is reading `oe_recent` with `JSON.parse` directly, which can still throw on malformed storage data and may return a non-array value that later breaks `recent.map(...)`. Update the `recent` initialization in `HomePage` to defensively parse the localStorage value, catch parse failures, and only keep it when the result is an array; otherwise fall back to an empty array.
8-9: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the import path casing.
These imports point at
@/components/home/..., but the files in this PR live undersrc/components/Home/.... That will fail on Linux CI and other case-sensitive builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` around lines 8 - 9, The imports in HomePage.jsx use the wrong folder casing, which will break on case-sensitive environments. Update the HeroSection and StatsSection import paths to match the actual src/components/Home directory casing exactly, and keep the component names unchanged so the module resolution works consistently across Linux CI and local builds.
77-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly route to
/overviewwhenexplore()succeeds.
src/context/AppContext.jsx:68-111returnsfalseafter setting error state on failed exploration. The unconditionalnavigate("/overview")here still redirects users to the overview page after a failed fetch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` around lines 77 - 79, The HomePage flow always navigates to /overview even when explore() fails, so update the logic around the explore(orgs) call and the navigate("/overview") in HomePage to only redirect when explore returns success. Check the boolean result from explore() (which AppContext sets to false on failure) and guard the navigation so failed fetches stay on the current page and preserve the error state.
11-11: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the quick-access labels.
quickExploreItemsis still hardcoding user-visible copy in code. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` at line 11, The quick-access labels in quickExploreItems are hardcoded user-visible strings and should be moved out of HomePage.jsx into the i18n resource files. Update HomePage and any related rendering logic to read these labels from the existing localization/resource layer instead of defining the array inline, keeping the quick explore item identifiers intact while externalizing the display copy.Source: Path instructions
33-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEncode the search term before building the GitHub URL.
debouncedQueryis still interpolated raw intoq=..., so spaces and reserved characters can produce malformed requests or wrong matches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` around lines 33 - 35, The GitHub search request in HomePage still interpolates debouncedQuery directly into the q parameter, which can break on spaces or reserved characters. Update the URL construction in HomePage’s fetch call to encode the search term before appending it to the GitHub search endpoint, and keep the rest of the query syntax intact so the request stays valid and matches correctly.src/components/Home/StatsSection.jsx (2)
11-45: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the stats copy.
All metric headings/descriptors in this section are still hardcoded literals. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/StatsSection.jsx` around lines 11 - 45, The metric headings and descriptors in StatsSection are still hardcoded user-facing strings. Externalize the copy used in the StatsSection component into the project’s i18n/resource files and replace the literals with translated lookups, keeping the existing visual structure and using the same keys wherever these labels are rendered.Source: Path instructions
15-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the
Girth With PATlabel.This still reads like a typo or placeholder and will ship directly to users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/StatsSection.jsx` around lines 15 - 16, The StatsSection label still shows a placeholder-like phrase and should be replaced before shipping. Update the text inside the StatsSection JSX paragraph in the Home stats component to a user-facing label that matches the intended product wording, and keep the change localized to that displayed string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.jsx`:
- Around line 5-6: Remove the unused `Navbar` and `RateLimitBanner` imports from
`App` in `src/App.jsx`, since `Layout` is already responsible for rendering
them. Keep the `App` component focused on its actual dependencies and delete
only the dead import statements so the module has no unused imports.
In `@src/components/layout/Footer.tsx`:
- Line 65: The Footer component’s <footer> element has a redundant contentinfo
role because the native footer tag already provides that landmark semantics.
Update the Footer component to remove the explicit role from the footer markup
and keep the existing className and structure unchanged.
In `@src/components/layout/Navbar.tsx`:
- Around line 25-27: Remove the redundant banner role from the Navbar header
element: the <header> in Navbar.tsx already provides banner semantics, so update
the header markup to rely on its native accessibility behavior and keep the
existing className styling unchanged.
- Around line 20-22: The mobile drawer state in Navbar stays open across route
transitions because Navbar remains mounted under Layout while Routes change.
Update Navbar so mobileMenuOpen is reset to false whenever the location changes,
and make sure the existing mobile nav/settings/logo click handlers also close
the drawer after navigating. Use the Navbar component’s state/hooks and the
related click handlers in the mobile menu section to implement the fix.
---
Outside diff comments:
In `@src/components/layout/Footer.tsx`:
- Around line 139-154: The Footer component still contains hardcoded user-facing
copy, so externalize the strings used in the Footer JSX instead of embedding
them directly. Update Footer.tsx to read the copyright line and the “Built for
open source communities” text from the project’s i18n/resource layer, using the
existing footer-related identifiers in the component to locate the text nodes.
Keep the component focused on rendering and ensure the localized keys are added
to the appropriate resource file so the footer can be translated cleanly.
In `@src/components/layout/Navbar.tsx`:
- Around line 33-35: The Navbar component is hardcoding user-facing copy,
including the homepage link ARIA label and other visible nav text, so it
bypasses localization. Move these strings out of Navbar.tsx into the app’s
i18n/resource files and read them through the existing translation mechanism.
Update the affected Navbar render paths and any related labels/menus in the same
component so all user-visible text is sourced from localized keys instead of
inline literals.
In `@src/pages/HomePage.jsx`:
- Line 1: The suggestion selection state in HomePage is not being kept in sync
with the filtered list, so `selectedIndex` can point past the end of
`filteredSuggestions` and crash when Enter reads from
`filteredSuggestions[selectedIndex]`. Update the `HomePage` logic that derives
or uses `selectedIndex` and `filteredSuggestions` to clamp the index back into
range or reset it whenever the suggestion list changes, especially in the
handler/effect around the search state and the Enter key path. Use the existing
`selectedIndex`, `filteredSuggestions`, and the input/menu navigation code to
ensure the active item always references a valid suggestion.
---
Duplicate comments:
In `@src/components/Home/QuickAccess.jsx`:
- Around line 6-8: The QuickAccess heading is still hardcoded as user-visible
text, so externalize it to the i18n/resource layer instead of rendering the
literal string in QuickAccess. Update the component to read the heading from the
existing translation/resource system used elsewhere in the app, and keep the
visible label unchanged through that localized key.
In `@src/components/Home/RecentSearches.jsx`:
- Around line 10-12: The section heading in RecentSearches is still hardcoded
and should be externalized for i18n. Update the RecentSearches component to
replace the literal “Recent Searches” with the appropriate localized resource
lookup used elsewhere in the app, so the heading is user-visible through the
resource file instead of inline text.
In `@src/components/Home/StatsSection.jsx`:
- Around line 11-45: The metric headings and descriptors in StatsSection are
still hardcoded user-facing strings. Externalize the copy used in the
StatsSection component into the project’s i18n/resource files and replace the
literals with translated lookups, keeping the existing visual structure and
using the same keys wherever these labels are rendered.
- Around line 15-16: The StatsSection label still shows a placeholder-like
phrase and should be replaced before shipping. Update the text inside the
StatsSection JSX paragraph in the Home stats component to a user-facing label
that matches the intended product wording, and keep the change localized to that
displayed string.
In `@src/pages/HomePage.jsx`:
- Line 25: The `HomePage` render path is reading `oe_recent` with `JSON.parse`
directly, which can still throw on malformed storage data and may return a
non-array value that later breaks `recent.map(...)`. Update the `recent`
initialization in `HomePage` to defensively parse the localStorage value, catch
parse failures, and only keep it when the result is an array; otherwise fall
back to an empty array.
- Around line 8-9: The imports in HomePage.jsx use the wrong folder casing,
which will break on case-sensitive environments. Update the HeroSection and
StatsSection import paths to match the actual src/components/Home directory
casing exactly, and keep the component names unchanged so the module resolution
works consistently across Linux CI and local builds.
- Around line 77-79: The HomePage flow always navigates to /overview even when
explore() fails, so update the logic around the explore(orgs) call and the
navigate("/overview") in HomePage to only redirect when explore returns success.
Check the boolean result from explore() (which AppContext sets to false on
failure) and guard the navigation so failed fetches stay on the current page and
preserve the error state.
- Line 11: The quick-access labels in quickExploreItems are hardcoded
user-visible strings and should be moved out of HomePage.jsx into the i18n
resource files. Update HomePage and any related rendering logic to read these
labels from the existing localization/resource layer instead of defining the
array inline, keeping the quick explore item identifiers intact while
externalizing the display copy.
- Around line 33-35: The GitHub search request in HomePage still interpolates
debouncedQuery directly into the q parameter, which can break on spaces or
reserved characters. Update the URL construction in HomePage’s fetch call to
encode the search term before appending it to the GitHub search endpoint, and
keep the rest of the query syntax intact so the request stays valid and matches
correctly.
🪄 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: bb4b19bc-020a-46e3-8461-7c2484d30958
📒 Files selected for processing (8)
src/App.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/StatsSection.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/pages/HomePage.jsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/HomePage.jsx (1)
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClamp
selectedIndexwhen the suggestion list changes.
selectedIndexsurvives query/result changes. If the list shrinks after the user has moved down the menu, Line 111 can readfilteredSuggestions[selectedIndex].loginfromundefinedand crash on Enter. Reset or clamp the index whenfilteredSuggestionschanges.🐛 Proposed fix
-import React, { useMemo, useState } from "react"; +import React, { useEffect, useMemo, useState } from "react"; @@ const filteredSuggestions = useMemo(() => { return suggestions.filter( (item) => item.login.toLowerCase() !== input.toLowerCase() ); }, [suggestions, input]); + + useEffect(() => { + setSelectedIndex((prev) => { + if (!filteredSuggestions.length) return -1; + return prev >= filteredSuggestions.length + ? filteredSuggestions.length - 1 + : prev; + }); + }, [filteredSuggestions.length]); @@ - if (e.key === "Enter" && selectedIndex >= 0) { + if ( + e.key === "Enter" && + selectedIndex >= 0 && + selectedIndex < filteredSuggestions.length + ) {Also applies to: 50-54, 108-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` at line 1, The suggestion selection state in HomePage is not being kept in sync with the filtered list, so `selectedIndex` can point past the end of `filteredSuggestions` and crash when Enter reads from `filteredSuggestions[selectedIndex]`. Update the `HomePage` logic that derives or uses `selectedIndex` and `filteredSuggestions` to clamp the index back into range or reset it whenever the suggestion list changes, especially in the handler/effect around the search state and the Enter key path. Use the existing `selectedIndex`, `filteredSuggestions`, and the input/menu navigation code to ensure the active item always references a valid suggestion.src/components/layout/Footer.tsx (1)
139-154: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the footer copy.
© 2026 OrgExplorerandBuilt for open source communitiesare still hardcoded in the component, so this footer cannot be localized cleanly. As per path instructions, "User-visible strings should be externalized to resource files (i18n)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Footer.tsx` around lines 139 - 154, The Footer component still contains hardcoded user-facing copy, so externalize the strings used in the Footer JSX instead of embedding them directly. Update Footer.tsx to read the copyright line and the “Built for open source communities” text from the project’s i18n/resource layer, using the existing footer-related identifiers in the component to locate the text nodes. Keep the component focused on rendering and ensure the localized keys are added to the appropriate resource file so the footer can be translated cleanly.Source: Path instructions
src/components/layout/Navbar.tsx (1)
33-35: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the new navbar copy.
This component hardcodes user-facing labels and ARIA text, which keeps the navigation out of the app’s localization path. Move the strings into resource files instead of embedding them in the component. As per path instructions, "User-visible strings should be externalized to resource files (i18n)."
Also applies to: 52-52, 92-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 33 - 35, The Navbar component is hardcoding user-facing copy, including the homepage link ARIA label and other visible nav text, so it bypasses localization. Move these strings out of Navbar.tsx into the app’s i18n/resource files and read them through the existing translation mechanism. Update the affected Navbar render paths and any related labels/menus in the same component so all user-visible text is sourced from localized keys instead of inline literals.Source: Path instructions
♻️ Duplicate comments (9)
src/components/Home/QuickAccess.jsx (1)
6-8: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the quick-access heading.
Quick Explore Accessis still hardcoded in the component. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/QuickAccess.jsx` around lines 6 - 8, The QuickAccess heading is still hardcoded as user-visible text, so externalize it to the i18n/resource layer instead of rendering the literal string in QuickAccess. Update the component to read the heading from the existing translation/resource system used elsewhere in the app, and keep the visible label unchanged through that localized key.Source: Path instructions
src/components/Home/RecentSearches.jsx (1)
10-12: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the section heading.
Recent Searchesis still a user-visible literal. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/RecentSearches.jsx` around lines 10 - 12, The section heading in RecentSearches is still hardcoded and should be externalized for i18n. Update the RecentSearches component to replace the literal “Recent Searches” with the appropriate localized resource lookup used elsewhere in the app, so the heading is user-visible through the resource file instead of inline text.Source: Path instructions
src/pages/HomePage.jsx (5)
25-25: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard and validate
oe_recentbefore render.A malformed value here still throws during render, and a valid-but-non-array payload will break later when
recent.map(...)runs. Parse this defensively and fall back to[]unless the result is actually an array.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` at line 25, The `HomePage` render path is reading `oe_recent` with `JSON.parse` directly, which can still throw on malformed storage data and may return a non-array value that later breaks `recent.map(...)`. Update the `recent` initialization in `HomePage` to defensively parse the localStorage value, catch parse failures, and only keep it when the result is an array; otherwise fall back to an empty array.
8-9: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the import path casing.
These imports point at
@/components/home/..., but the files in this PR live undersrc/components/Home/.... That will fail on Linux CI and other case-sensitive builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` around lines 8 - 9, The imports in HomePage.jsx use the wrong folder casing, which will break on case-sensitive environments. Update the HeroSection and StatsSection import paths to match the actual src/components/Home directory casing exactly, and keep the component names unchanged so the module resolution works consistently across Linux CI and local builds.
77-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly route to
/overviewwhenexplore()succeeds.
src/context/AppContext.jsx:68-111returnsfalseafter setting error state on failed exploration. The unconditionalnavigate("/overview")here still redirects users to the overview page after a failed fetch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` around lines 77 - 79, The HomePage flow always navigates to /overview even when explore() fails, so update the logic around the explore(orgs) call and the navigate("/overview") in HomePage to only redirect when explore returns success. Check the boolean result from explore() (which AppContext sets to false on failure) and guard the navigation so failed fetches stay on the current page and preserve the error state.
11-11: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the quick-access labels.
quickExploreItemsis still hardcoding user-visible copy in code. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` at line 11, The quick-access labels in quickExploreItems are hardcoded user-visible strings and should be moved out of HomePage.jsx into the i18n resource files. Update HomePage and any related rendering logic to read these labels from the existing localization/resource layer instead of defining the array inline, keeping the quick explore item identifiers intact while externalizing the display copy.Source: Path instructions
33-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEncode the search term before building the GitHub URL.
debouncedQueryis still interpolated raw intoq=..., so spaces and reserved characters can produce malformed requests or wrong matches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/HomePage.jsx` around lines 33 - 35, The GitHub search request in HomePage still interpolates debouncedQuery directly into the q parameter, which can break on spaces or reserved characters. Update the URL construction in HomePage’s fetch call to encode the search term before appending it to the GitHub search endpoint, and keep the rest of the query syntax intact so the request stays valid and matches correctly.src/components/Home/StatsSection.jsx (2)
11-45: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExternalize the stats copy.
All metric headings/descriptors in this section are still hardcoded literals. As per path instructions, "User-visible strings should be externalized to resource files (i18n)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/StatsSection.jsx` around lines 11 - 45, The metric headings and descriptors in StatsSection are still hardcoded user-facing strings. Externalize the copy used in the StatsSection component into the project’s i18n/resource files and replace the literals with translated lookups, keeping the existing visual structure and using the same keys wherever these labels are rendered.Source: Path instructions
15-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the
Girth With PATlabel.This still reads like a typo or placeholder and will ship directly to users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/StatsSection.jsx` around lines 15 - 16, The StatsSection label still shows a placeholder-like phrase and should be replaced before shipping. Update the text inside the StatsSection JSX paragraph in the Home stats component to a user-facing label that matches the intended product wording, and keep the change localized to that displayed string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.jsx`:
- Around line 5-6: Remove the unused `Navbar` and `RateLimitBanner` imports from
`App` in `src/App.jsx`, since `Layout` is already responsible for rendering
them. Keep the `App` component focused on its actual dependencies and delete
only the dead import statements so the module has no unused imports.
In `@src/components/layout/Footer.tsx`:
- Line 65: The Footer component’s <footer> element has a redundant contentinfo
role because the native footer tag already provides that landmark semantics.
Update the Footer component to remove the explicit role from the footer markup
and keep the existing className and structure unchanged.
In `@src/components/layout/Navbar.tsx`:
- Around line 25-27: Remove the redundant banner role from the Navbar header
element: the <header> in Navbar.tsx already provides banner semantics, so update
the header markup to rely on its native accessibility behavior and keep the
existing className styling unchanged.
- Around line 20-22: The mobile drawer state in Navbar stays open across route
transitions because Navbar remains mounted under Layout while Routes change.
Update Navbar so mobileMenuOpen is reset to false whenever the location changes,
and make sure the existing mobile nav/settings/logo click handlers also close
the drawer after navigating. Use the Navbar component’s state/hooks and the
related click handlers in the mobile menu section to implement the fix.
---
Outside diff comments:
In `@src/components/layout/Footer.tsx`:
- Around line 139-154: The Footer component still contains hardcoded user-facing
copy, so externalize the strings used in the Footer JSX instead of embedding
them directly. Update Footer.tsx to read the copyright line and the “Built for
open source communities” text from the project’s i18n/resource layer, using the
existing footer-related identifiers in the component to locate the text nodes.
Keep the component focused on rendering and ensure the localized keys are added
to the appropriate resource file so the footer can be translated cleanly.
In `@src/components/layout/Navbar.tsx`:
- Around line 33-35: The Navbar component is hardcoding user-facing copy,
including the homepage link ARIA label and other visible nav text, so it
bypasses localization. Move these strings out of Navbar.tsx into the app’s
i18n/resource files and read them through the existing translation mechanism.
Update the affected Navbar render paths and any related labels/menus in the same
component so all user-visible text is sourced from localized keys instead of
inline literals.
In `@src/pages/HomePage.jsx`:
- Line 1: The suggestion selection state in HomePage is not being kept in sync
with the filtered list, so `selectedIndex` can point past the end of
`filteredSuggestions` and crash when Enter reads from
`filteredSuggestions[selectedIndex]`. Update the `HomePage` logic that derives
or uses `selectedIndex` and `filteredSuggestions` to clamp the index back into
range or reset it whenever the suggestion list changes, especially in the
handler/effect around the search state and the Enter key path. Use the existing
`selectedIndex`, `filteredSuggestions`, and the input/menu navigation code to
ensure the active item always references a valid suggestion.
---
Duplicate comments:
In `@src/components/Home/QuickAccess.jsx`:
- Around line 6-8: The QuickAccess heading is still hardcoded as user-visible
text, so externalize it to the i18n/resource layer instead of rendering the
literal string in QuickAccess. Update the component to read the heading from the
existing translation/resource system used elsewhere in the app, and keep the
visible label unchanged through that localized key.
In `@src/components/Home/RecentSearches.jsx`:
- Around line 10-12: The section heading in RecentSearches is still hardcoded
and should be externalized for i18n. Update the RecentSearches component to
replace the literal “Recent Searches” with the appropriate localized resource
lookup used elsewhere in the app, so the heading is user-visible through the
resource file instead of inline text.
In `@src/components/Home/StatsSection.jsx`:
- Around line 11-45: The metric headings and descriptors in StatsSection are
still hardcoded user-facing strings. Externalize the copy used in the
StatsSection component into the project’s i18n/resource files and replace the
literals with translated lookups, keeping the existing visual structure and
using the same keys wherever these labels are rendered.
- Around line 15-16: The StatsSection label still shows a placeholder-like
phrase and should be replaced before shipping. Update the text inside the
StatsSection JSX paragraph in the Home stats component to a user-facing label
that matches the intended product wording, and keep the change localized to that
displayed string.
In `@src/pages/HomePage.jsx`:
- Line 25: The `HomePage` render path is reading `oe_recent` with `JSON.parse`
directly, which can still throw on malformed storage data and may return a
non-array value that later breaks `recent.map(...)`. Update the `recent`
initialization in `HomePage` to defensively parse the localStorage value, catch
parse failures, and only keep it when the result is an array; otherwise fall
back to an empty array.
- Around line 8-9: The imports in HomePage.jsx use the wrong folder casing,
which will break on case-sensitive environments. Update the HeroSection and
StatsSection import paths to match the actual src/components/Home directory
casing exactly, and keep the component names unchanged so the module resolution
works consistently across Linux CI and local builds.
- Around line 77-79: The HomePage flow always navigates to /overview even when
explore() fails, so update the logic around the explore(orgs) call and the
navigate("/overview") in HomePage to only redirect when explore returns success.
Check the boolean result from explore() (which AppContext sets to false on
failure) and guard the navigation so failed fetches stay on the current page and
preserve the error state.
- Line 11: The quick-access labels in quickExploreItems are hardcoded
user-visible strings and should be moved out of HomePage.jsx into the i18n
resource files. Update HomePage and any related rendering logic to read these
labels from the existing localization/resource layer instead of defining the
array inline, keeping the quick explore item identifiers intact while
externalizing the display copy.
- Around line 33-35: The GitHub search request in HomePage still interpolates
debouncedQuery directly into the q parameter, which can break on spaces or
reserved characters. Update the URL construction in HomePage’s fetch call to
encode the search term before appending it to the GitHub search endpoint, and
keep the rest of the query syntax intact so the request stays valid and matches
correctly.
🪄 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: bb4b19bc-020a-46e3-8461-7c2484d30958
📒 Files selected for processing (8)
src/App.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/StatsSection.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/pages/HomePage.jsx
🛑 Comments failed to post (4)
src/App.jsx (1)
5-6: 🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -e echo "App imports:" sed -n '1,20p' src/App.jsx echo echo "Layout ownership:" sed -n '1,30p' src/components/layout/Layout.tsx echo echo "Navbar component files under src/components:" fd -i 'Navbar\.(js|jsx|ts|tsx)$' src/components echo echo "RateLimitBanner component files under src/components:" fd -i 'RateLimitBanner\.(js|jsx|ts|tsx)$' src/componentsRepository: AOSSIE-Org/OrgExplorer
Length of output: 1654
Remove unused imports for Navbar and RateLimitBanner
The imports for
NavbarandRateLimitBannerinsrc/App.jsxare unused becauseLayouthandles rendering these components. While the files still exist, the imports constitute dead code and should be removed.Code snippet
- import Navbar from './components/Navbar' - import RateLimitBanner from './components/RateLimitBanner'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.jsx` around lines 5 - 6, Remove the unused `Navbar` and `RateLimitBanner` imports from `App` in `src/App.jsx`, since `Layout` is already responsible for rendering them. Keep the `App` component focused on its actual dependencies and delete only the dead import statements so the module has no unused imports.src/components/layout/Footer.tsx (1)
65-65: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the redundant contentinfo role.
<footer>already providescontentinfosemantics, so the explicit role is unnecessary.🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 65-65: Screen reader users gain nothing from this
rolebecause<footer>already acts as acontentinfo, so remove it.Remove redundant
roleattributes so assistive tech reads the element's native semantics without extra noise.(no-redundant-roles)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Footer.tsx` at line 65, The Footer component’s <footer> element has a redundant contentinfo role because the native footer tag already provides that landmark semantics. Update the Footer component to remove the explicit role from the footer markup and keep the existing className and structure unchanged.Source: Linters/SAST tools
src/components/layout/Navbar.tsx (2)
20-22: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Close the mobile drawer on route changes.
Navbaris rendered byLayout, so it stays mounted while<Routes>switches pages. Right now a mobile navigation/settings/logo click leavesmobileMenuOpenset totrue, and the destination page opens with the drawer still expanded.Suggested fix
-import { useState } from "react"; -import { Link } from "react-router-dom"; +import { useEffect, useState } from "react"; +import { Link, useLocation } from "react-router-dom"; @@ export default function Navbar() { const [mobileMenuOpen, setMobileMenuOpen] = useState(false); + const location = useLocation(); const { rateLimit } = useApp(); const lowLimit = rateLimit && rateLimit.remaining < 15 + + useEffect(() => { + setMobileMenuOpen(false); + }, [location.pathname]);Also applies to: 89-99, 106-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 20 - 22, The mobile drawer state in Navbar stays open across route transitions because Navbar remains mounted under Layout while Routes change. Update Navbar so mobileMenuOpen is reset to false whenever the location changes, and make sure the existing mobile nav/settings/logo click handlers also close the drawer after navigating. Use the Navbar component’s state/hooks and the related click handlers in the mobile menu section to implement the fix.
25-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the redundant banner role.
<header>already exposes banner semantics, sorole="banner"adds noise without improving accessibility.🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 26-26: Screen reader users gain nothing from this
rolebecause<header>already acts as abanner, so remove it.Remove redundant
roleattributes so assistive tech reads the element's native semantics without extra noise.(no-redundant-roles)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 25 - 27, Remove the redundant banner role from the Navbar header element: the <header> in Navbar.tsx already provides banner semantics, so update the header markup to rely on its native accessibility behavior and keep the existing className styling unchanged.Source: Linters/SAST tools
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.
Screenshots/Recordings:
Before
https://1drv.ms/v/c/2b55451e453f4443/IQD4zvJn69ISQruWI4E14WXIAQljG07WmHb-TaKpKqNgpp4?e=c4ijFd
After
https://1drv.ms/v/c/2b55451e453f4443/IQByP7pqDHsET7G91kJmi_IxAfANKsKW7nRMgHpA7hfT-SE?e=4JtfEa
https://1drv.ms/v/c/2b55451e453f4443/IQBgpGkq-3QTSoKzBkoxJn_vAdySS0yXVy7QuPg1XKzmcGk?e=zGqXgc
Additional Notes:
Improved UI as per recommendation.
refactor: restructure Navbar and layout components
chore: set up TypeScript configuration
style: update global styles and integrate Tailwind CSS
fix: integrate React Query for data fetching
test: add utility functions and hooks
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
New Features
Bug Fixes