feat: add multiple tech stack selection in filters #182
Conversation
- Replace radio buttons with checkboxes for tech stack filter when allowMultiple is true - Add updateMultipleFilters method to store to handle array of selected values - Update Filter component to support both single and multiple selection modes - Modify UserInputFilterProps type to accept string or string[] for Tech stack - Update converter logic to handle multiple languages by joining them with space - Enable allowMultiple prop for Tech stack filter in FiltersContainer This allows users to select multiple programming languages simultaneously when searching for projects, providing more flexible filtering options.
|
Someone is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces multi-select capability for the "Tech stack" filter. The Filter component now conditionally renders checkboxes when Changes
Sequence DiagramsequenceDiagram
participant User
participant Filter Component
participant Store
participant Converter
User->>Filter Component: Click checkbox (allowMultiple=true)
Filter Component->>Store: updateMultipleFilters("Tech stack", values[])
Store->>Store: Merge values into filters
Store-->>Filter Component: State updated
alt Single Selection Path
User->>Filter Component: Select radio button
Filter Component->>Store: updateFilters("Tech stack", value)
Store->>Store: Replace filter value
end
User->>Converter: Trigger conversion
Converter->>Converter: Check if "Tech stack" is array
alt Array Path
Converter->>Converter: Join array with spaces
else String Path
Converter->>Converter: Use string directly
end
Converter-->>User: API-formatted output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
user.name.like : "barnalixd seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/src/store/useFilterInputStore.ts (1)
4-4: Improve type safety for the filters state.The
filtersproperty is typed as genericobject, which provides no type safety. Consider usingUserInputFilterPropsorPartial<UserInputFilterProps>from@/types/filterto ensure type-safe filter operations.+import { UserInputFilterProps } from "@/types/filter"; + interface FilterInputState { - filters: object; + filters: Partial<UserInputFilterProps>; updateFilters: (newFilter: Record<string, string>) => void; updateMultipleFilters: (filterName: string, values: string[]) => void; resetFilters: () => void; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/components/ui/Filter.tsx(3 hunks)apps/web/src/components/ui/FiltersContainer.tsx(1 hunks)apps/web/src/store/useFilterInputStore.ts(2 hunks)apps/web/src/types/filter.ts(1 hunks)apps/web/src/utils/converter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/ui/Filter.tsx (1)
apps/web/src/store/useFilterInputStore.ts (1)
useFilterInputStore(10-21)
🔇 Additional comments (7)
apps/web/src/components/ui/FiltersContainer.tsx (1)
103-103: LGTM! Multi-select enabled for Tech stack filter.The
allowMultiple={true}prop correctly enables checkbox-based multi-selection for the Tech stack filter, aligning with the PR objective.apps/web/src/store/useFilterInputStore.ts (1)
6-6: LGTM! TheupdateMultipleFiltersmethod is correctly implemented.The new method properly merges multiple filter values into the state, supporting the multi-select functionality.
Also applies to: 16-19
apps/web/src/types/filter.ts (2)
51-51: LGTM! Type correctly supports both single and multiple selections.Changing
"Tech stack"fromstringtostring | string[]properly enables the filter to handle both single-value and multi-value selections.
55-55: Good catch! Missing Activity field added.The
Activityfield was already being used inapps/web/src/utils/converter.ts(lines 134-137) but was missing from the type definition. This addition fixes a type inconsistency, though it appears unrelated to the multi-select feature.apps/web/src/components/ui/Filter.tsx (2)
23-45: LGTM! Multi-select logic is well-structured.The
recordFilterInputfunction correctly handles both single and multiple selection modes:
- In multi-select mode, it maintains an array and toggles items in/out
- In single-select mode, it preserves the original behavior
- Empty array handling is appropriate
47-53: LGTM! Checkbox state helper is correct.The
isCheckedfunction properly determines whether a filter option should be checked in multi-select mode.apps/web/src/utils/converter.ts (1)
114-122: Verify GitHub API syntax for multiple languages and fix backend coordination.The review comment correctly identifies two real issues:
GitHub API syntax error: The correct syntax for multiple programming languages requires repeated
language:qualifiers (e.g.,language:python language:javascript), not space-separated values. The current code produces"javascript typescript"which won't work.Empty array handling: An empty array
[]is truthy, so the code would setdata.language = "", creating an invalid querylanguage:.However, the suggested fix in the review would create a different problem. The backend code (apps/api/src/services/project.service.ts:31) already adds the
language:prefix withqueryParts.push(\language:${filters.language}`). If the frontend sends"language:javascript language:typescript", the backend would wrap it again, creatinglanguage:language:javascript language:typescript"` (double prefix).The fix requires backend coordination:
- Either the backend should be modified to handle arrays of languages or detect pre-formatted query strings
- Or the frontend should pass languages differently (not as a single space-separated string)
- The empty array check is still needed regardless
Recommend verifying the intended architecture with the backend implementation to ensure multiple language support works correctly.
| <Checkbox | ||
| id={filter} | ||
| checked={isChecked(filter)} | ||
| onCheckedChange={() => recordFilterInput(filter)} | ||
| className="border-[#28282c] bg-[#141418] text-ox-purple transition data-[state=checked]:border-ox-purple data-[state=checked]:bg-ox-purple/20 data-[state=checked]:ring-2 data-[state=checked]:ring-ox-purple/50" | ||
| /> | ||
| <Label | ||
| htmlFor={filter} | ||
| onClick={() => recordFilterInput(filter)} | ||
| className="text-sm text-zinc-300 cursor-pointer transition-colors" | ||
| > | ||
| {filter} | ||
| </Label> |
There was a problem hiding this comment.
Critical: Double-triggering due to redundant onClick handlers.
Both the Checkbox component and the Label are calling recordFilterInput(filter):
- Line 75:
CheckboxhasonCheckedChange={() => recordFilterInput(filter)} - Lines 78-80:
Labelhas bothhtmlFor={filter}ANDonClick={() => recordFilterInput(filter)}
When a user clicks the label:
- The
htmlForassociation triggers the checkbox, firingonCheckedChange - Then the label's
onClickfires, callingrecordFilterInputagain - This causes the selection to toggle twice, reverting to the original state
Remove the onClick handler from the Label since htmlFor already provides the click association:
<Label
htmlFor={filter}
- onClick={() => recordFilterInput(filter)}
className="text-sm text-zinc-300 cursor-pointer transition-colors"
>
{filter}
</Label>📝 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.
| <Checkbox | |
| id={filter} | |
| checked={isChecked(filter)} | |
| onCheckedChange={() => recordFilterInput(filter)} | |
| className="border-[#28282c] bg-[#141418] text-ox-purple transition data-[state=checked]:border-ox-purple data-[state=checked]:bg-ox-purple/20 data-[state=checked]:ring-2 data-[state=checked]:ring-ox-purple/50" | |
| /> | |
| <Label | |
| htmlFor={filter} | |
| onClick={() => recordFilterInput(filter)} | |
| className="text-sm text-zinc-300 cursor-pointer transition-colors" | |
| > | |
| {filter} | |
| </Label> | |
| <Checkbox | |
| id={filter} | |
| checked={isChecked(filter)} | |
| onCheckedChange={() => recordFilterInput(filter)} | |
| className="border-[#28282c] bg-[#141418] text-ox-purple transition data-[state=checked]:border-ox-purple data-[state=checked]:bg-ox-purple/20 data-[state=checked]:ring-2 data-[state=checked]:ring-ox-purple/50" | |
| /> | |
| <Label | |
| htmlFor={filter} | |
| className="text-sm text-zinc-300 cursor-pointer transition-colors" | |
| > | |
| {filter} | |
| </Label> |
🤖 Prompt for AI Agents
In apps/web/src/components/ui/Filter.tsx around lines 72 to 84, the Label and
Checkbox both invoke recordFilterInput(filter) causing double-triggering when
the label is clicked; remove the Label's onClick handler and rely on htmlFor to
forward clicks to the Checkbox (keep Checkbox's onCheckedChange as the single
source of truth), and ensure the Label retains htmlFor and appropriate classes
for accessibility and styling.
| <RadioGroupItem | ||
| value={filter} | ||
| id={filter} | ||
| onClick={() => recordFilterInput(filter)} | ||
| className="border-[#28282c] bg-[#141418] text-ox-purple transition data-[state=checked]:border-ox-purple data-[state=checked]:bg-ox-purple/20 data-[state=checked]:ring-2 data-[state=checked]:ring-ox-purple/50" | ||
| /> | ||
| <Label | ||
| htmlFor={filter} | ||
| onClick={() => recordFilterInput(filter)} | ||
| className="text-sm text-zinc-300 cursor-pointer transition-colors" | ||
| > | ||
| {filter} | ||
| </Label> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify: Potential double-triggering in radio button mode.
Similar to the checkbox issue, both the RadioGroupItem and Label have click handlers:
- Line 95:
RadioGroupItemhasonClick={() => recordFilterInput(filter)} - Lines 98-100:
Labelhas bothhtmlFor={filter}ANDonClick={() => recordFilterInput(filter)}
If this pattern existed in the original code and was working, it may be that the radio group implementation prevents double-firing. However, for consistency with the checkbox fix and to avoid potential issues, consider removing the redundant onClick from the label here as well.
Verify whether the radio button implementation was working correctly before this PR. If there were no issues, document why; otherwise, apply the same fix:
<Label
htmlFor={filter}
- onClick={() => recordFilterInput(filter)}
className="text-sm text-zinc-300 cursor-pointer transition-colors"
>
{filter}
</Label>I'll help you verify this review comment. Let me gather information about the radio button implementation and compare it with the checkbox pattern.
Now let me examine the Filter.tsx file to understand the full context and see how the checkbox issue was handled:
#!/bin/bash
# Examine Filter.tsx around the radio button lines and look for checkbox pattern
fd -e tsx -e ts -path "*/Filter.tsx" -x cat {} | head -150
---
I need to gather more information to verify this concern properly.
🏁 Script executed:
#!/bin/bash
# Find and examine Filter.tsx file, focusing on radio and checkbox patterns
find . -name "Filter.tsx" -o -name "filter.tsx" 2>/dev/null | head -5 | xargs -I {} bash -c 'echo "=== File: {} ===" && cat {}'Length of output: 4204
---
🌐 Web query:
```web
radix-ui RadioGroup RadioGroupItem onClick event handler
💡 Result:
Short answer
- Use RadioGroup.Root's onValueChange to react to selection changes (it receives the selected value string). [1][2]
- You can still add an onClick to individual RadioGroup.Item (it's rendered as a button), but selection logic / change events should come from Root (Item no longer has onCheckedChange). [1][2]
Example (React + Radix)
-
Preferred: handle selection at the group level
const handleChange = (value: string) => console.log('selected', value);
<RadioGroup.Root onValueChange={handleChange} defaultValue="a">
<RadioGroup.Item value="a" id="a" />A
<RadioGroup.Item value="b" id="b" />B
</RadioGroup.Root> -
Per-item click (allowed, but keep selection handling on Root)
<RadioGroup.Item value="a" onClick={() => console.log('item A clicked')} />
Notes
- Radix changed the API: onValueChange now receives the value string, and RadioGroup.Item's onCheckedChange prop was removed — prefer Root-level onValueChange for change handling. [2]
Sources
[1] Radix Radio Group docs. [2] Radix releases (Radio Group notes).
Remove duplicate onClick from Label in both Checkbox and RadioGroupItem sections.
Based on radix-ui documentation and the code inspection, I found that:
-
RadioGroupItem pattern (lines 87–94): Both
RadioGroupItemandLabelhaveonClick={() => recordFilterInput(filter)}. When the label is clicked viahtmlFor, the Label's onClick fires, and this likely causesrecordFilterInputto be called. -
Confirmed issue in Checkbox pattern (lines 65–71): The same double-handler structure exists here too. The
CheckboxhasonCheckedChange(proper radix-ui event), but theLabelalso hasonClick. When the label is clicked, both fire, causing a confirmed double-trigger. -
Standards alignment: Per radix-ui documentation, the preferred pattern is to use
RadioGroup.Root'sonValueChangehandler for selection logic, notonClickon individual items. Similarly,Checkboxshould rely solely ononCheckedChange.
Remove the onClick handler from the Label in both sections to prevent duplicate calls:
<Label
htmlFor={filter}
- onClick={() => recordFilterInput(filter)}
className="text-sm text-zinc-300 cursor-pointer transition-colors"
>Apply this fix to:
- Line 69 (Checkbox Label)
- Line 92 (RadioGroupItem Label)
📝 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.
| <RadioGroupItem | |
| value={filter} | |
| id={filter} | |
| onClick={() => recordFilterInput(filter)} | |
| className="border-[#28282c] bg-[#141418] text-ox-purple transition data-[state=checked]:border-ox-purple data-[state=checked]:bg-ox-purple/20 data-[state=checked]:ring-2 data-[state=checked]:ring-ox-purple/50" | |
| /> | |
| <Label | |
| htmlFor={filter} | |
| onClick={() => recordFilterInput(filter)} | |
| className="text-sm text-zinc-300 cursor-pointer transition-colors" | |
| > | |
| {filter} | |
| </Label> | |
| <RadioGroupItem | |
| value={filter} | |
| id={filter} | |
| onClick={() => recordFilterInput(filter)} | |
| className="border-[#28282c] bg-[#141418] text-ox-purple transition data-[state=checked]:border-ox-purple data-[state=checked]:bg-ox-purple/20 data-[state=checked]:ring-2 data-[state=checked]:ring-ox-purple/50" | |
| /> | |
| <Label | |
| htmlFor={filter} | |
| className="text-sm text-zinc-300 cursor-pointer transition-colors" | |
| > | |
| {filter} | |
| </Label> |
🤖 Prompt for AI Agents
In apps/web/src/components/ui/Filter.tsx around lines 65–71 and 87–94
(specifically remove the Label onClick at line 69 for the Checkbox block and at
line 92 for the RadioGroupItem block): the Label currently duplicates click
handling already handled by the Checkbox (onCheckedChange) and
RadioGroupItem/RadioGroup.Root (onValueChange), causing double-triggered events;
remove the onClick={() => recordFilterInput(filter)} prop from both Label
elements so selection logic relies only on the input components' handlers (keep
htmlFor and styling intact).
|
hey @BARNALIXD could you please share a working screen recording also please look at the coderabbit reviews! |
2 participants
This allows users to select multiple programming languages simultaneously when searching for projects, providing more flexible filtering options.
Summary by CodeRabbit
Release Notes