Add clear filters functionlity, update filters styles #232
Conversation
…ems, improving layout and usability
WalkthroughThe pull request introduces several modifications across multiple components, primarily focusing on the user interface and structure of various elements. Key changes include the removal of the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
app/sections/collection-filters/sort.tsx (1)
1-1: Remove unused importThe
CheckCircleicon is imported but never used in the code.-import { CaretDown, CheckCircle } from "@phosphor-icons/react"; +import { CaretDown } from "@phosphor-icons/react";app/sections/collection-filters/products-pagination.tsx (1)
41-64: Add accessibility improvements to filter tagsThe filter tags implementation looks good but could benefit from accessibility enhancements.
<Link key={label} to={getAppliedFilterLink(filter, params, location)} - className="px-2 py-1 border border-line-subtle hover:border-line items-center gap-2" + className="px-2 py-1 border border-line-subtle hover:border-line items-center gap-2 inline-flex" + aria-label={`Remove ${label} filter`} variant="custom" > <span>{label}</span> - <X className="w-4 h-4" /> + <X className="w-4 h-4" aria-hidden="true" /> </Link>app/sections/collection-filters/filters.tsx (5)
47-49: Consider using filter IDs instead of labels for key generation.The current implementation concatenates filter labels to generate a key. Using filter IDs would be more reliable as labels might not be unique or could contain special characters.
- let appliedFiltersKeys = appliedFilters - .map((filter) => filter.label) - .join("-"); + let appliedFiltersKeys = appliedFilters + .map((filter) => filter.id) + .join("-");
56-56: Optimize Accordion key generation.Including
expandFiltersandshowFiltersCountin the key is unnecessary as these props will trigger re-renders naturally when changed. Consider using only theappliedFiltersKeys.- key={appliedFiltersKeys + expandFilters.toString() + showFiltersCount} + key={appliedFiltersKeys}
Line range hint
141-232: Consider splitting FilterItem into separate components.The component has grown complex with multiple display types. Consider:
- Splitting into separate components (ColorSwatchFilter, ButtonFilter, ListItemFilter).
- Extracting button styles to a constant.
This would improve maintainability and make the code more modular.
Example refactor:
const buttonStyles = { base: "px-3 py-1.5 border text-center", disabled: "diagonal text-body-subtle", checked: "border-line bg-body text-background", unchecked: "border-line-subtle hover:border-line", } as const; function ButtonFilter({ option, checked, onChange, showFiltersCount }) { return ( <button type="button" className={cn( buttonStyles.base, option.count === 0 && buttonStyles.disabled, checked ? buttonStyles.checked : buttonStyles.unchecked )} onClick={onChange} > <FilterLabel option={option} showFiltersCount={showFiltersCount} /> </button> ); } // Similar components for ColorSwatchFilter and ListItemFilter
246-258: Consider memoizing FilterLabel for performance.Since FilterLabel is used across all filter types and re-renders frequently, consider memoizing it with React.memo to prevent unnecessary re-renders.
-function FilterLabel({ +const FilterLabel = React.memo(function FilterLabel({ option, showFiltersCount, }: { option: Filter["values"][0]; showFiltersCount: boolean; }) { if (showFiltersCount) { return ( <span> {option.label} <span>({option.count})</span> </span> ); } return option.label; -} +});
Clear filters functionality is partially implemented but needs proper handling
The "Clear all filters" link exists in
products-pagination.tsx, but it only uses a basic navigation to the pathname without any filter state management. For proper implementation:
- The
Linkcomponent inproducts-pagination.tsxneeds to be enhanced with proper filter state clearing functionality- Consider adding a handler to properly reset all filter states when clicked
🔗 Analysis chain
Line range hint
1-1: Missing clear filters functionality mentioned in PR title.The PR title mentions "Add clear filters functionality" but the implementation doesn't include this feature. Please clarify if this functionality will be added in a separate PR or if it should be implemented here.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any clear filters related code rg -i "clear.?filter" --type ts --type tsxLength of output: 72
Script:
#!/bin/bash # Search for any clear filters related code, using ripgrep without file type restrictions rg -i "clear.?filter" # Also search for potential clear/reset functionality with different naming rg -i "(clear|reset).*(filter|selection)" # Look for filter-related components and their props/methods ast-grep --pattern 'function $_($$$ filter $$$) { $$$ }'Length of output: 214
Script:
#!/bin/bash # Let's check the context around the "Clear all filters" text to understand its implementation rg -B 5 -A 5 "Clear all filters" app/sections/collection-filters/products-pagination.tsx # Look for any filter-related state management or handlers ast-grep --pattern 'const $_ = $$$filter$$$' # Check for any onClick or similar handlers that might be related to clearing filters rg -i "on.*click.*clear" -A 2Length of output: 593
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
app/components/header/desktop-header.tsx(0 hunks)app/components/tooltip.tsx(1 hunks)app/sections/collection-filters/filters.tsx(7 hunks)app/sections/collection-filters/products-pagination.tsx(2 hunks)app/sections/collection-filters/sort.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- app/components/header/desktop-header.tsx
🔇 Additional comments (6)
app/components/tooltip.tsx (1)
33-33: LGTM: Minor padding adjustment
The horizontal padding reduction from px-4 to px-3 is a safe UI refinement.
app/sections/collection-filters/sort.tsx (2)
72-72: LGTM: Improved sort indicator
Changed from underline to bold for better visual feedback of the selected sort option.
4-4: Verify Link component behavior
The import source for Link has changed from @remix-run/react to a custom component. Ensure the custom Link component maintains the same navigation behavior.
✅ Verification successful
Custom Link component maintains proper navigation behavior
The custom Link component is a well-implemented wrapper around Remix's Link component (RemixLink). It:
- Properly imports and extends Remix's Link functionality
- Adds useful features like locale prefixing and styling variants
- Forwards all standard Link props to the underlying RemixLink component
- Maintains the core navigation behavior through proper prop forwarding
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the custom Link component properly handles navigation
ast-grep --pattern 'export default function Link({ $props }) {
$$$
}'
Length of output: 73
Script:
#!/bin/bash
# Let's first find the custom Link component file
fd "link.tsx" --type f
# Then examine its implementation
rg -l "export.*function.*Link" --type ts --type tsx
# Look for any usage of Remix's Link component
rg "import.*Link.*from.*@remix-run/react" --type ts --type tsx
Length of output: 249
Script:
#!/bin/bash
# Let's examine the implementation of the custom Link component
cat app/components/link.tsx
# Look for any usage of Remix's Link component without file type flags
rg "import.*Link.*from.*@remix-run/react"
Length of output: 7580
app/sections/collection-filters/products-pagination.tsx (2)
29-34: LGTM: Type-safe loader data
Good use of TypeScript to ensure type safety with the added appliedFilters data.
60-62: LGTM: Clear filters functionality
The "Clear all filters" link implementation aligns with the PR objectives and provides a good user experience.
app/sections/collection-filters/filters.tsx (1)
62-62: LGTM! Clean implementation of display type handling.
The conditional className logic effectively handles different display types while maintaining clean code structure.
Also applies to: 92-94
Summary by CodeRabbit
New Features
DialogDemocomponent.CheckCircleicon in the sorting component.Bug Fixes
Documentation