-
Notifications
You must be signed in to change notification settings - Fork 155
feat: add multiple tech stack selection in filters #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AccordionContent, AccordionItem, AccordionTrigger } from "./accordion"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Checkbox } from "@/components/ui/checkbox"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Label } from "@/components/ui/label"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useFilterInputStore } from "@/store/useFilterInputStore"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import clsx from "clsx"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -10,16 +11,45 @@ export default function Filter({ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| filterName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| filters, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowMultiple = false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| filterName: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| filters: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick?: () => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowMultiple?: boolean; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { updateFilters } = useFilterInputStore(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const inputData: { [key: string]: string } = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { updateFilters, updateMultipleFilters, filters: currentFilters } = useFilterInputStore(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const recordFilterInput = (filter: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputData[filterName] = filter; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateFilters(inputData); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (allowMultiple) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle multiple selection for Tech stack | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const currentSelected = (currentFilters[filterName] as string[]) || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isSelected = currentSelected.includes(filter); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| let newSelected: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isSelected) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Remove from selection | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| newSelected = currentSelected.filter((item) => item !== filter); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add to selection | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| newSelected = [...currentSelected, filter]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateMultipleFilters(filterName, newSelected); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle single selection (original behavior) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const inputData: { [key: string]: string } = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputData[filterName] = filter; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateFilters(inputData); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isChecked = (filter: string): boolean => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (allowMultiple) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const currentSelected = (currentFilters[filterName] as string[]) || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return currentSelected.includes(filter); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const triggerClasses = clsx("text-sm font-medium", { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -35,25 +65,47 @@ export default function Filter({ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="text-sm font-medium text-white">{filterName}</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </AccordionTrigger> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AccordionContent className="pt-1 pb-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <RadioGroup className="space-y-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| {filters.map((filter) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={filter} className="flex items-center space-x-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <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> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </RadioGroup> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| {allowMultiple ? ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="space-y-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| {filters.map((filter) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={filter} className="flex items-center space-x-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <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> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) : ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <RadioGroup className="space-y-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| {filters.map((filter) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={filter} className="flex items-center space-x-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| <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> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+92
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify: Potential double-triggering in radio button mode. Similar to the checkbox issue, both the
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 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🏁 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 💡 Result: Short answer
Example (React + Radix)
Notes
Sources Remove duplicate Based on radix-ui documentation and the code inspection, I found that:
Remove the <Label
htmlFor={filter}
- onClick={() => recordFilterInput(filter)}
className="text-sm text-zinc-300 cursor-pointer transition-colors"
>Apply this fix to:
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </RadioGroup> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </AccordionContent> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </AccordionItem> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Double-triggering due to redundant onClick handlers.
Both the
Checkboxcomponent and theLabelare callingrecordFilterInput(filter):CheckboxhasonCheckedChange={() => recordFilterInput(filter)}Labelhas bothhtmlFor={filter}ANDonClick={() => recordFilterInput(filter)}When a user clicks the label:
htmlForassociation triggers the checkbox, firingonCheckedChangeonClickfires, callingrecordFilterInputagainRemove the
onClickhandler from theLabelsincehtmlForalready provides the click association:<Label htmlFor={filter} - onClick={() => recordFilterInput(filter)} className="text-sm text-zinc-300 cursor-pointer transition-colors" > {filter} </Label>📝 Committable suggestion
🤖 Prompt for AI Agents