Feat/home page#65
Conversation
WalkthroughThis PR standardizes formatting/tooling, modernizes build (TypeScript + Vite TS config), introduces UI primitives and utilities, extracts a Layout (Navbar/Footer) and updates App, and refactors Home into component-driven search UI with debounced GitHub suggestions. ChangesBuild tooling and code formatting setup
UI component foundation and utilities
Layout component system and app refactor
Home page search and component-driven layout
Pages and services formatting; NetworkPage caution
Sequence Diagram(s) sequenceDiagram
participant User
participant HeroSection
participant ReactQuery as useQuery/QueryClient
participant Fetch as fetchWithCache (github.js)
participant IndexedDB
User->>HeroSection: type query / press Enter
HeroSection->>ReactQuery: trigger useQuery (debounced)
ReactQuery->>Fetch: call fetchWithCache(search users)
Fetch->>IndexedDB: cacheGet(url) / cacheSet(url,data)
Fetch->>ReactQuery: return suggestions
ReactQuery->>HeroSection: deliver isLoading + suggestions
HeroSection->>User: render suggestions / select
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 |
|
@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 |
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
| group: github-pages | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Review concurrency setting change.
The concurrency group changed from pages to github-pages and cancel-in-progress was changed from false to true. Setting cancel-in-progress: true for deployment workflows can be risky because it may cancel an in-progress deployment, potentially leaving the site in an inconsistent state. Consider whether canceling deployments mid-flight is safe for your use case.
🔧 Recommended: Restore safer concurrency behavior
concurrency:
group: github-pages
- cancel-in-progress: true
+ cancel-in-progress: false📝 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.
| group: github-pages | |
| cancel-in-progress: true | |
| group: github-pages | |
| cancel-in-progress: false |
🤖 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 @.github/workflows/deploy.yml around lines 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.
| globalIgnores(["dist"]), | ||
| { | ||
| files: ['**/*.{ts,tsx}'], | ||
| files: ["**/*.{ts,tsx}"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm there are still JSX/JS sources that ESLint would currently skip.
fd -e jsx -e js --type f src | head -50
echo "---- counts ----"
echo "jsx: $(fd -e jsx --type f src | wc -l) js: $(fd -e js --type f src | wc -l) ts/tsx: $(fd -e ts -e tsx --type f src | wc -l)"Repository: AOSSIE-Org/OrgExplorer
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== eslint.config.js (relevant lines) =="
ls -la
sed -n '1,120p' eslint.config.js | nl -ba | sed -n '1,120p'
echo
echo "== JS/JSX file counts across repo =="
echo "jsx: $(fd -e jsx --type f . | wc -l)"
echo "js: $(fd -e js --type f . | wc -l)"
echo "ts: $(fd -e ts --type f . | wc -l)"
echo "tsx: $(fd -e tsx --type f . | wc -l)"
echo
echo "== sample JS/JSX files (up to 20 each) =="
echo "--- jsx ---"
fd -e jsx --type f . | head -20
echo "--- js ---"
fd -e js --type f . | head -20Repository: AOSSIE-Org/OrgExplorer
Length of output: 2155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== eslint.config.js =="
grep -n "files:" -n eslint.config.js || true
sed -n '1,140p' eslint.config.js | cat -n
echo
echo "== JS/JSX counts (repo root) =="
echo "jsx: $(fd -e jsx --type f . | wc -l)"
echo "js: $(fd -e js --type f . | wc -l)"
echo "ts: $(fd -e ts --type f . | wc -l)"
echo "tsx: $(fd -e tsx --type f . | wc -l)"
echo
echo "== sample .jsx files =="
fd -e jsx --type f . | head -20 || true
echo "== sample .js files =="
fd -e js --type f . | head -20 || trueRepository: AOSSIE-Org/OrgExplorer
Length of output: 1711
Expand ESLint file globs to include existing JS/JSX
eslint.config.js scopes the config to files: ["**/*.{ts,tsx}"], so current src/**/*.jsx and src/**/*.js sources (e.g., src/App.jsx, src/hooks/useSortedData.js) are not linted, leaving React-hooks/React Refresh rules and JS errors undetected there.
♻️ Suggested glob widening
- files: ["**/*.{ts,tsx}"],
+ files: ["**/*.{ts,tsx,js,jsx}"],📝 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.
| files: ["**/*.{ts,tsx}"], | |
| files: ["**/*.{ts,tsx,js,jsx}"], |
🤖 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 `@eslint.config.js` at 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.
| <p className="mt-2 text-sm font-semibold uppercase text-white"> | ||
| Girth With PAT | ||
| </p> |
There was a problem hiding this comment.
Fix the user-facing label “Girth With PAT”.
This reads as a placeholder/typo rather than a real metric description and will be visible to end users. Replace it with the intended copy (e.g. “Repos With PAT” or similar).
🤖 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 - 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.
| {footerLinks.map((item) => ( | ||
| <Link | ||
| key={item.label} | ||
| to={item.href} | ||
| className=" | ||
| text-xs font-medium uppercase tracking-[0.2em] | ||
| text-zinc-500 | ||
| transition-colors duration-200 | ||
| hover:text-[#FCD34D] | ||
| focus-visible:outline-none | ||
| focus-visible:ring-2 | ||
| focus-visible:ring-[#FCD34D] | ||
| focus-visible:ring-offset-2 | ||
| focus-visible:ring-offset-black | ||
| rounded-sm | ||
| " | ||
| > | ||
| {item.label} | ||
| </Link> | ||
| ))} |
There was a problem hiding this comment.
Footer navigation links point to unregistered routes. /docs, /terms, /privacy, and /status are not defined in src/App.jsx's routes, so the catch-all <Route path="*" element={<Navigate to="/" replace />} /> will silently redirect all of these to the home page—broken UX. Either add the corresponding routes/pages or replace these with external <a> links to the real destinations.
🤖 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, 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.
| throw new Error( | ||
| "No valid organizations found. Check the names and try again." | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Externalize user-visible strings for internationalization.
Error messages are hardcoded in English and should be externalized to resource files to support i18n. Examples:
- "No valid organizations found. Check the names and try again."
- "GitHub API rate limit reached. Add a PAT in Settings for 5,000 req/hr."
As per coding guidelines: "User-visible strings should be externalized to resource files (i18n)."
Also applies to: 89-93
🤖 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/context/AppContext.jsx` around lines 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.
| import HeroSection from "@/components/home/HeroSection"; | ||
| import StatsSection from "@/components/home/StatsSection"; | ||
| import OrgExplorerFeatures from "@/components/home/OrgExplorerFeatures"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the actual directory casing for the Home components.
fd -t d -i '^home$' src/components
fd -i 'HeroSection|StatsSection|OrgExplorerFeatures' src/componentsRepository: AOSSIE-Org/OrgExplorer
Length of output: 204
Fix import path casing in src/pages/HomePage.jsx (Linux CI case-sensitive builds)
Imports currently use @/components/home/..., but the actual components live in src/components/Home/ (capital H). Update the import paths to match.
🐛 Proposed fix
-import HeroSection from "`@/components/home/HeroSection`";
-import StatsSection from "`@/components/home/StatsSection`";
-import OrgExplorerFeatures from "`@/components/home/OrgExplorerFeatures`";
+import HeroSection from "`@/components/Home/HeroSection`";
+import StatsSection from "`@/components/Home/StatsSection`";
+import OrgExplorerFeatures from "`@/components/Home/OrgExplorerFeatures`";📝 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.
| import HeroSection from "@/components/home/HeroSection"; | |
| import StatsSection from "@/components/home/StatsSection"; | |
| import OrgExplorerFeatures from "@/components/home/OrgExplorerFeatures"; | |
| import HeroSection from "`@/components/Home/HeroSection`"; | |
| import StatsSection from "`@/components/Home/StatsSection`"; | |
| import OrgExplorerFeatures from "`@/components/Home/OrgExplorerFeatures`"; |
🤖 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 - 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.
|
|
||
| const debouncedQuery = useDebounce(input, 1000); | ||
|
|
||
| const recent = JSON.parse(localStorage.getItem("oe_recent") || "[]"); |
There was a problem hiding this comment.
Guard JSON.parse against malformed localStorage data.
If oe_recent is ever corrupted, JSON.parse throws during render and crashes the page. Wrap it in a safe parse.
🛡️ Proposed fix
- const recent = JSON.parse(localStorage.getItem("oe_recent") || "[]");
+ let recent = [];
+ try {
+ recent = JSON.parse(localStorage.getItem("oe_recent") || "[]");
+ } catch {
+ recent = [];
+ }📝 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.
| const recent = JSON.parse(localStorage.getItem("oe_recent") || "[]"); | |
| let recent = []; | |
| try { | |
| recent = JSON.parse(localStorage.getItem("oe_recent") || "[]"); | |
| } catch { | |
| recent = []; | |
| } |
🤖 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 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.
| const response = await fetch( | ||
| `https://api.github.com/search/users?q=${debouncedQuery}+type:org` | ||
| ); |
There was a problem hiding this comment.
Encode the query parameter.
debouncedQuery is interpolated raw into the URL. Inputs with spaces or special characters (#, &, ?, etc.) produce malformed requests and incorrect results. Use encodeURIComponent.
🐛 Proposed fix
const response = await fetch(
- `https://api.github.com/search/users?q=${debouncedQuery}+type:org`
+ `https://api.github.com/search/users?q=${encodeURIComponent(debouncedQuery)}+type:org`
);📝 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.
| const response = await fetch( | |
| `https://api.github.com/search/users?q=${debouncedQuery}+type:org` | |
| ); | |
| const response = await fetch( | |
| `https://api.github.com/search/users?q=${encodeURIComponent(debouncedQuery)}+type:org` | |
| ); |
🤖 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 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.
| @keyframes spin { | ||
| to { transform: rotate(360deg); } | ||
| to { | ||
| transform: rotate(360deg); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider using kebab-case for keyframe names.
The fadeUp keyframe uses camelCase, which deviates from the kebab-case convention commonly used in CSS. Consider renaming to fade-up for consistency with CSS naming patterns.
♻️ Proposed fix
-@keyframes fadeUp {
+@keyframes fade-up {
from {
opacity: 0;
transform: translateY(10px);
}
to {
opacity: 1;
transform: translateY(0);
}
}
.fade-up {
- animation: fadeUp 0.22s ease;
+ animation: fade-up 0.22s ease;
}As per coding guidelines: "The code adheres to best practices associated with CSS."
Also applies to: 80-82
🤖 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 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.
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
| aria-label="Search GitHub organization" | ||
| placeholder="AOSSIE-Org, StabilityNexus, DjedAlliance.." |
There was a problem hiding this comment.
Externalize user-visible strings for internationalization.
The aria-label and placeholder attributes contain hardcoded user-facing strings. As per coding guidelines, all user-visible strings should be externalized to i18n resource files.
🌐 Example i18n implementation
Assuming an i18n setup like react-i18next:
+import { useTranslation } from 'react-i18next';
+
export default function OrgSearchBox({
// ... props
}) {
+ const { t } = useTranslation();
+
return (
<div className="relative mt-12 w-full max-w-3xl">
<form
// ...
>
<div className="relative flex flex-1 items-center gap-5 rounded-xl border border-zinc-800 bg-black px-4 py-4">
// ...
<input
id="org-search"
type="text"
value={input}
autoComplete="off"
- aria-label="Search GitHub organization"
- placeholder="AOSSIE-Org, StabilityNexus, DjedAlliance.."
+ aria-label={t('search.ariaLabel')}
+ placeholder={t('search.placeholder')}
// ...
/>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` around lines 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.
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
Refactor
Chores