Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 75 additions & 23 deletions apps/web/src/components/ui/Filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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", {
Expand All @@ -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>
Comment on lines +72 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Double-triggering due to redundant onClick handlers.

Both the Checkbox component and the Label are calling recordFilterInput(filter):

  1. Line 75: Checkbox has onCheckedChange={() => recordFilterInput(filter)}
  2. Lines 78-80: Label has both htmlFor={filter} AND onClick={() => recordFilterInput(filter)}

When a user clicks the label:

  • The htmlFor association triggers the checkbox, firing onCheckedChange
  • Then the label's onClick fires, calling recordFilterInput again
  • 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.

Suggested change
<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.

</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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify: Potential double-triggering in radio button mode.

Similar to the checkbox issue, both the RadioGroupItem and Label have click handlers:

  1. Line 95: RadioGroupItem has onClick={() => recordFilterInput(filter)}
  2. Lines 98-100: Label has both htmlFor={filter} AND onClick={() => 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:

  1. RadioGroupItem pattern (lines 87–94): Both RadioGroupItem and Label have onClick={() => recordFilterInput(filter)}. When the label is clicked via htmlFor, the Label's onClick fires, and this likely causes recordFilterInput to be called.

  2. Confirmed issue in Checkbox pattern (lines 65–71): The same double-handler structure exists here too. The Checkbox has onCheckedChange (proper radix-ui event), but the Label also has onClick. When the label is clicked, both fire, causing a confirmed double-trigger.

  3. Standards alignment: Per radix-ui documentation, the preferred pattern is to use RadioGroup.Root's onValueChange handler for selection logic, not onClick on individual items. Similarly, Checkbox should rely solely on onCheckedChange.

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.

Suggested change
<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).

</div>
))}
</RadioGroup>
)}
</AccordionContent>
</AccordionItem>
</div>
Expand Down
1 change: 1 addition & 0 deletions apps/web/src/components/ui/FiltersContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export default function FiltersContainer() {
"Html",
"Elixir",
]}
allowMultiple={true}
/>
<Filter
filterName="Popularity"
Expand Down
5 changes: 5 additions & 0 deletions apps/web/src/store/useFilterInputStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { create } from "zustand";
interface FilterInputState {
filters: object;
updateFilters: (newFilter: Record<string, string>) => void;
updateMultipleFilters: (filterName: string, values: string[]) => void;
resetFilters: () => void;
}

Expand All @@ -12,5 +13,9 @@ export const useFilterInputStore = create<FilterInputState>((set) => ({
set((state) => ({
filters: { ...state.filters, ...newFilter },
})),
updateMultipleFilters: (filterName, values) =>
set((state) => ({
filters: { ...state.filters, [filterName]: values },
})),
resetFilters: () => set({ filters: {} }),
}));
2 changes: 1 addition & 1 deletion apps/web/src/types/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export type ProjectProps = {
};

export type UserInputFilterProps = {
"Tech stack"?: string;
"Tech stack"?: string | string[];
Popularity?: string;
Competition?: string;
Stage?: string;
Expand Down
8 changes: 7 additions & 1 deletion apps/web/src/utils/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,13 @@ export const convertUserInputToApiInput = (
const data: Partial<FilterProps> = {};

if (filter["Tech stack"]) {
data.language = filter["Tech stack"];
// Handle both single string and array of strings
if (Array.isArray(filter["Tech stack"])) {
// Join multiple languages with space (GitHub search syntax)
data.language = filter["Tech stack"].join(" ");
} else {
data.language = filter["Tech stack"];
}
}

if (filter.Popularity) {
Expand Down