SearchPage sort dropdown uses native select instead of custom dropdown#1491
SearchPage sort dropdown uses native select instead of custom dropdown#1491shineetejol9 wants to merge 4 commits into
Conversation
|
@shineetejol9 is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
📝 WalkthroughWalkthroughThis PR adds an optional analysis timeout and reuses earlier language detection in repository analysis, updates Prisma datasource/model and package versions, fixes Breadcrumbs JSX, revises Dashboard keyboard focus and loading skeletons, and replaces the SearchPage sort select with a DropdownMenu. ChangesAnalysis Service & Dashboard Improvements
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/repositoryService.ts (1)
429-429:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
opts.scopewhen detecting languages.
getFileTreestill scopes toopts?.scope, butdetectLanguagesnow always scopes torepository.targetDirectory. Scoped analyses can therefore persist files for one subtree and language percentages for another.Suggested fix
const [contributors, languages] = await Promise.all([ gitService.getContributors(), gitService.detectLanguages({ - targetDirectory: repository.targetDirectory ?? null, + targetDirectory: opts?.scope ?? repository.targetDirectory ?? null, }), ]);Also applies to: 474-478
🤖 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 `@lib/services/repositoryService.ts` at line 429, The language detection now ignores the requested scope: ensure detectLanguages is called with the same scope used for gitService.getFileTree by passing opts?.scope (or a scoped path variable derived from it) instead of unconditionally using repository.targetDirectory; update the calls to detectLanguages (the one at the shown getFileTree usage and the other occurrence around the later block) to accept and use opts?.scope so file persistence and language percentages remain consistent with the scoped analysis.
🤖 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.
Outside diff comments:
In `@lib/services/repositoryService.ts`:
- Line 429: The language detection now ignores the requested scope: ensure
detectLanguages is called with the same scope used for gitService.getFileTree by
passing opts?.scope (or a scoped path variable derived from it) instead of
unconditionally using repository.targetDirectory; update the calls to
detectLanguages (the one at the shown getFileTree usage and the other occurrence
around the later block) to accept and use opts?.scope so file persistence and
language percentages remain consistent with the scoped analysis.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b731244-c0a9-493a-9d3b-034e466e9ed3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
lib/services/repositoryService.tspackage.jsonprisma/schema.prismasrc/components/layout/Breadcrumbs.tsxsrc/pages/Dashboard.tsx
shineetejol9
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/pages/SearchPage.tsx`:
- Around line 163-169: The sort trigger Button in SearchPage.tsx currently only
shows the selected value via the sortBy state; update the Button (the Button
element rendering the sort trigger) to include an explicit label for
accessibility by adding a descriptive aria-label (e.g., aria-label={`Sort
results by ${sortBy}`} or a visible label element tied to the control) so screen
readers understand the control purpose while preserving the existing displayed
value; ensure the label updates with sortBy and does not alter Button styling or
behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f75aafe7-a84f-4a0c-9b64-5c9c4c4b63d9
📒 Files selected for processing (1)
src/pages/SearchPage.tsx
| <Button variant="outline" size="sm"> | ||
| {sortBy === "recent" | ||
| ? "Recent" | ||
| : sortBy === "stars" | ||
| ? "Most Stars" | ||
| : "Name"} | ||
| </Button> |
There was a problem hiding this comment.
Make the sort trigger explicitly labeled for accessibility.
On Line 163, the button text only reflects the selected value; it doesn’t clearly expose the control purpose (sort) to all users. Add an explicit label (visual or aria-label) so the trigger is self-descriptive.
Suggested patch
- <Button variant="outline" size="sm">
- {sortBy === "recent"
- ? "Recent"
- : sortBy === "stars"
- ? "Most Stars"
- : "Name"}
+ <Button
+ variant="outline"
+ size="sm"
+ aria-label="Sort repositories"
+ >
+ Sort:{" "}
+ {sortBy === "recent"
+ ? "Recent"
+ : sortBy === "stars"
+ ? "Most Stars"
+ : "Name"}
</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.
| <Button variant="outline" size="sm"> | |
| {sortBy === "recent" | |
| ? "Recent" | |
| : sortBy === "stars" | |
| ? "Most Stars" | |
| : "Name"} | |
| </Button> | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| aria-label="Sort repositories" | |
| > | |
| Sort:{" "} | |
| {sortBy === "recent" | |
| ? "Recent" | |
| : sortBy === "stars" | |
| ? "Most Stars" | |
| : "Name"} | |
| </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/pages/SearchPage.tsx` around lines 163 - 169, The sort trigger Button in
SearchPage.tsx currently only shows the selected value via the sortBy state;
update the Button (the Button element rendering the sort trigger) to include an
explicit label for accessibility by adding a descriptive aria-label (e.g.,
aria-label={`Sort results by ${sortBy}`} or a visible label element tied to the
control) so screen readers understand the control purpose while preserving the
existing displayed value; ensure the label updates with sortBy and does not
alter Button styling or behavior.
|
Hey @shineetejol9! Saw your work on GSSoC 2026. We are building TermUI, a TypeScript terminal UI framework with React-style hooks and JSX, rendered entirely in the terminal. We have 67 unassigned GSSoC issues open. 19 are marked Karanjot, TermUI maintainer |
2 participants
Description
Replaced the native HTML select element used for repository sorting in SearchPage with the project's custom DropdownMenu component. This change improves UI consistency across the application and aligns the sorting control with the existing design system while preserving the existing sorting functionality.
Related Issue
Closes # 710
Type of Change
Screenshots
Add screenshots or recordings for UI changes. Write
N/Aif this pull request does not change the UI.Testing
Describe the commands you ran and any manual verification performed.
npm run lintnpm run buildnpm run formatgit diff --checkN/Afor documentation-only changesN/Awith a reasonChecklist
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Chores