Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request enhances the email search functionality by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SB as SearchBar Component
participant UF as Utility Functions
U->>SB: Enter search query
SB->>UF: matchFilterPrefix(input)
UF-->>SB: Return prefix and suggestion data
SB->>UF: filterSuggestionsFunction(suggestions, prefix, query)
UF-->>SB: Return filtered suggestions
SB->>SB: Update state & render suggestions
U->>SB: Navigate using keyboard (handleKeyDown)
SB-->>U: Update active suggestion/highlight
Poem
✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
apps/mail/lib/utils.ts (1)
82-87: Add JSDoc or comments to describe the new type for maintainability.The
FilterSuggestiontype is straightforward and well-structured. For clarity, consider briefly documenting what each property represents (e.g.,description,iconusage, etc.). This helps future contributors quickly grasp each field's purpose.apps/mail/components/mail/search-bar.tsx (8)
21-23: Promote unified import statements for clarity.Imports from
@/lib/utilscould be grouped or commented to highlight which items are types vs. which are utility functions. Not a strong requirement, just a minor clarity suggestion (e.g., separate import lines for types).
42-47: Regex might be too restrictive for certain user inputs.
matchFilterPrefixleverages/(is|has|from|to):(\w*)$/which ignores special characters (like.or+). If you anticipate addresses like "from:someone+test@example.com," this won’t fully match.- const match = /(is|has|from|to):(\w*)$/.exec(text); + const match = /(is|has|from|to):([^ ]*)$/.exec(text);Consider broadening the capture group if you need to match more complex addresses.
49-71: Flexible filtering function.
filterSuggestionsFunctionlogically returns filter suggestions by matching prefix and partial value. The approach is clean and easy to extend. Ensure you have tests verifying edge cases such as emptyprefix, partial matches, and uppercase/lowercase.Would you like me to open a new issue to add or refine tests for these edge cases?
75-94: Well-defined suggestion array.The
filterSuggestionsarray covers core scenarios (is:,has:,from:,to:). As your product grows, consider modularizing or loading these from a config or server-driven data.
190-219: Streamlined suggestion logic inhandleInputChange.The function expertly checks if
matchFilterPrefixis triggered and updates the suggestion state. This is well-structured. Consider clearing suggestions if the user removes punctuation mid-typing.
285-295: Click-outside effect.Using a global
mousedownlistener is effective. Just ensure it doesn’t conflict with other suggestion popovers in the app. Potentially could be replaced by a more generic or library-based approach if re-used in multiple places.
301-361:renderSuggestionsfactoring is clean and test-friendly.Rendering logic is neatly encapsulated. Consider adding ARIA roles or labels for accessibility, especially for screen readers.
404-413: Efficient string building insubmitSearch.Combining multiple filters (from, to, subject, category) is concise. There's a small risk of extra spacing if some are empty. You could
.trim()once after concatenation.Also applies to: 421-421
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/mail/search-bar.tsx(5 hunks)apps/mail/lib/utils.ts(1 hunks)
🔇 Additional comments (13)
apps/mail/lib/utils.ts (1)
90-97:Details
❓ Verification inconclusive
Consider handling multiple colons or special characters.
extractFilterValuecurrently uses the first colon encountered, which works for basic prefixes (e.g.,from:someone). However, if a user includes more complex IDs or addresses with special characters (e.g., “from:someone+test@example.com”), the(is|has|from|to):(\w*)pattern or subsequent usage may not fully extract the intended filter value.Would you like me to search for references to ensure multi-colon or special character usage is not required?
🏁 Script executed:
#!/bin/bash # This script searches for references to `extractFilterValue` and checks # for possible usage scenarios with colons or special characters. rg -A 4 'extractFilterValue'Length of output: 1084
Attention: Verify Special Character and Multi-Colon Handling in Filter Extraction
Currently,
extractFilterValueretrieves everything after the first colon, which aligns with its sole usage in the search bar. However, if filter strings might include multiple colons or special characters (e.g.,"from:someone+test@example.com"), please manually verify that this simple extraction meets all expected input cases or consider enhancing the logic if needed.
- File:
apps/mail/lib/utils.ts(lines 90–97)- Usage: Check that its usage in
apps/mail/components/mail/search-bar.tsxcorrectly handles inputs with extra colons or non-alphanumeric characters.apps/mail/components/mail/search-bar.tsx (12)
16-16: Memoization usage looks good.Importing and using
useState,useEffect,useRef,useCallback, anduseMemoindicates a careful performance-oriented approach. This is a positive pattern in React for optimizing re-renders.
26-40: Appropriately sized grid columns for filter suggestions.
getFilterSuggestionGridColumnselegantly scales columns based on count and viewport (mobile). The return logic is clear and maintainable.
165-169: State variables for suggestion dropdown.
[showSuggestions, filteredSuggestions, activeSuggestionIndex, activePrefix]are well named and keep the search bar’s logic straightforward. No immediate issues here.
176-178: Memoizing the watched “q” value.Storing
qinformValueswithuseMemoavoids repeated re-renders. Confirm thatform.watch('q')alone re-renders only as intended.
180-188:filteringcomputed property.The combined check for
q,from,to,dateRange,category, andfolderis neat. This helps conditionally show reset buttons, etc. Looks good.
221-283: Keyboard navigation carefully handled.Tab/arrow key handling, especially with multi-column navigation, is an excellent user experience approach. The logic is a bit extensive; ensure thorough testing for corner cases (e.g., last row, single suggestion, etc.).
297-300: Responsive suggestion grid columns.Reusing
getFilterSuggestionGridColumnswithuseMemois well done for performance. This ensures suggestions adapt quickly without extra overhead.
362-387: Appending space after applying a suggestion.
handleSuggestionClickinserts the chosen filter plus a trailing space, which is typically user-friendly. Just watch out for double spaces if the user already typed one.
423-423: Reset withresetSearch.Fully resets the form and search context. This is a good approach for clearing state. No concerns here.
441-443: Inline callback references.You're directly passing
handleInputChangeandhandleKeyDown. This is consistent. Just confirm that these are stable references due touseCallback; otherwise, the input might re-render more than necessary.
445-445: Conditional suggestion rendering.Invoking
renderSuggestions()directly is readable and keeps JSX tidy. No issues detected.
9-9:Details
✅ Verification successful
Ensure imported icons are actually used.
A large number of icons from "lucide-react" are imported. Verify if all are utilized, or consider removing unused ones to keep the bundle size small.
🏁 Script executed:
#!/bin/bash # Searches for references to these icon imports in the file: rg -A 2 'Search|SlidersHorizontal|CalendarIcon|Trash2|Tag|Bell|AlertTriangle|User|Mail|MailQuestion|UserRound|Users|Star|MailCheck|Paperclip' apps/mail/components/mail/search-bar.tsxLength of output: 4405
All imported icons are currently in use.
After reviewing the file at
apps/mail/components/mail/search-bar.tsx, all icons from thelucide-reactimport are referenced throughout the component (e.g., within the search input, filter configurations, and buttons). No unused icons were found during the verification.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
apps/mail/components/mail/search-bar.tsx (8)
9-9: Consider code splitting to reduce bundle size.The line includes a large number of icon imports from Lucide, which could potentially impact the initial load time.
You could use dynamic imports to load icons on demand, especially for ones that are only used conditionally:
-import { Search, SlidersHorizontal, CalendarIcon, Trash2, Tag, Bell, AlertTriangle, User, Mail, MailQuestion, UserRound, Users, Star, MailCheck, Paperclip, X } from "lucide-react"; +import { Search, SlidersHorizontal, CalendarIcon, X } from "lucide-react"; +// Group less frequently used icons for potential code splitting +import { Trash2, Tag, Bell, AlertTriangle, User, Mail, MailQuestion, UserRound, Users, Star, MailCheck, Paperclip } from "lucide-react";Alternatively, consider lazy loading the entire SearchBar component since it contains numerous icons and complex UI elements.
16-16: Destructure only the hooks you need from React.Good job adding required hooks, but it's best to import only what you need.
-import { useState, useEffect, useRef, useCallback, useMemo } from "react"; +import { useState, useEffect, useRef, useCallback, useMemo } from "react";
42-47: Regex pattern could be more efficient.The regex pattern works correctly but could be slightly optimized.
Consider compiling the regex pattern once outside the function to avoid recompilation on each call:
+const FILTER_PREFIX_REGEX = /(is|has|from|to|after|before):([^\s]*)$/; const matchFilterPrefix = (text: string): [string, string, string] | null => { - const match = /(is|has|from|to|after|before):([^\s]*)$/.exec(text); + const match = FILTER_PREFIX_REGEX.exec(text); if (!match || !match[1]) return null; return [match[0], match[1], match[2] || '']; };
197-207: Consider consolidating related state.There are multiple closely related state variables that could be consolidated for better state management.
Consider grouping related state into a single object:
- const [showSuggestions, setShowSuggestions] = useState(false); - const [filteredSuggestions, setFilteredSuggestions] = useState<FilterSuggestion[]>([]); - const [activeSuggestionIndex, setActiveSuggestionIndex] = useState(0); - const [activePrefix, setActivePrefix] = useState<string | null>(null); + const [suggestionsState, setSuggestionsState] = useState({ + show: false, + filtered: [] as FilterSuggestion[], + activeIndex: 0, + activePrefix: null as string | null + }); + - const [showDatePicker, setShowDatePicker] = useState(false); - const [dateFilterType, setDateFilterType] = useState<'after' | 'before' | null>(null); - const [datePickerPosition, setDatePickerPosition] = useState({ left: 0, top: 0 }); + const [datePickerState, setDatePickerState] = useState({ + show: false, + filterType: null as 'after' | 'before' | null, + position: { left: 0, top: 0 } + });This would simplify state updates and make related state changes more atomic.
226-284: DOM manipulation for positioning could be improved.The DOM manipulation for positioning the date picker is complex and potentially fragile.
Consider using a library like Popper.js or a React positioning hook for more reliable positioning:
if (prefix === 'after' || prefix === 'before') { setDateFilterType(prefix as 'after' | 'before'); - const inputEl = inputRef.current; - if (inputEl) { - const span = document.createElement('span'); - span.style.visibility = 'hidden'; - span.style.position = 'absolute'; - span.style.whiteSpace = 'pre'; - span.style.font = window.getComputedStyle(inputEl).font; - span.textContent = textBeforeCursor; - document.body.appendChild(span); - - const rect = inputEl.getBoundingClientRect(); - const spanWidth = span.getBoundingClientRect().width; - - document.body.removeChild(span); - - setDatePickerPosition({ - left: Math.min(spanWidth, rect.width - 320), - top: rect.height - }); - } + // Use the input's position directly + if (inputRef.current) { + const rect = inputRef.current.getBoundingClientRect(); + const cursorPosition = inputRef.current.selectionStart || 0; + + // Estimate the position based on character count + const averageCharWidth = 8; // Approximate width in pixels + const estimatedCursorX = Math.min(cursorPosition * averageCharWidth, rect.width - 320); + + setDatePickerPosition({ + left: estimatedCursorX, + top: rect.height + }); + }Or better yet, consider using a UI library with positioned components that handle this complexity for you.
438-486: Date handling logic could be simplified.The date selection handling has some redundancy that could be simplified.
const handleDateSelect = useCallback((dateRange: DateRange | undefined) => { if (!dateRange || !dateFilterType) return; - let filterText = ''; + const dateParts = []; - if (dateFilterType === 'after' && dateRange.from) { - const formattedDate = format(dateRange.from, "yyyy/MM/dd"); - filterText = `after:${formattedDate}`; - - if (dateRange.to) { - const formattedEndDate = format(dateRange.to, "yyyy/MM/dd"); - filterText += ` before:${formattedEndDate}`; - } - } else if (dateFilterType === 'before' && dateRange.to) { - const formattedDate = format(dateRange.to, "yyyy/MM/dd"); - filterText = `before:${formattedDate}`; - - if (dateRange.from) { - const formattedStartDate = format(dateRange.from, "yyyy/MM/dd"); - filterText = `after:${formattedStartDate} before:${formattedDate}`; - } + // Always add 'after' if we have a from date + if (dateRange.from) { + const formattedStartDate = format(dateRange.from, "yyyy/MM/dd"); + dateParts.push(`after:${formattedStartDate}`); + } + + // Always add 'before' if we have a to date + if (dateRange.to) { + const formattedEndDate = format(dateRange.to, "yyyy/MM/dd"); + dateParts.push(`before:${formattedEndDate}`); } + + const filterText = dateParts.join(' '); if (!filterText) return;
502-567: Memoization needed for renderSuggestions.The renderSuggestions function is correctly using useCallback, but has a potential performance issue.
The function re-calculates gridColumns even though it's already calculated in the component. Reuse the memoized value:
- const gridColumns = getFilterSuggestionGridColumns(filteredSuggestions.length, isMobile); + // Use the already memoized gridColumns value
569-595: Consider componentizing complex UI elements.The renderDatePicker function creates a fairly complex UI element that could be extracted into its own component.
Consider moving this to a separate component to improve code maintainability:
// In a separate file: DatePickerPopover.tsx interface DatePickerPopoverProps { show: boolean; position: { left: number; top: number }; isMobile: boolean; onSelect: (dateRange: DateRange | undefined) => void; } export function DatePickerPopover({ show, position, isMobile, onSelect }: DatePickerPopoverProps) { const datePickerRef = useRef<HTMLDivElement>(null); if (!show) return null; return ( <div ref={datePickerRef} className="absolute z-50 mt-1 overflow-hidden rounded-lg border border-border bg-background shadow-md animate-in fade-in-50 slide-in-from-top-2 duration-150" style={{ left: Math.max(0, position.left - (isMobile ? 160 : 320)), top: `${position.top}px`, }} > <div className="p-1"> <Calendar initialFocus mode="range" defaultMonth={new Date()} selected={undefined} onSelect={onSelect} numberOfMonths={isMobile ? 1 : 2} disabled={(date) => date > new Date()} className="rounded-md border-none" /> </div> </div> ); }Then in your main component:
// In SearchBar.tsx const renderDatePicker = useCallback(() => { return ( <DatePickerPopover show={showDatePicker} position={datePickerPosition} isMobile={isMobile} onSelect={handleDateSelect} /> ); }, [showDatePicker, datePickerPosition, handleDateSelect, isMobile]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/mail/search-bar.tsx(5 hunks)
🔇 Additional comments (8)
apps/mail/components/mail/search-bar.tsx (8)
21-21: Good addition of the new type and utilities from the shared library.Adding the FilterSuggestion type import helps standardize the structure of filter suggestions across components.
26-40: Well-structured responsive grid calculation function.This utility function elegantly handles grid column sizing based on the number of items and device type. The responsive design considerations are good.
103-126: Comprehensive set of filter suggestions.Good job creating a complete set of filter suggestions with appropriate icons and descriptions. The commented sections provide context for future development.
163-163: Good use of responsive hook for calendar.Using the
useIsMobilehook to dynamically adjust the number of displayed months in the calendar is a nice touch for responsive design.
216-224: Good use of useMemo for derived state.Using useMemo to compute the filtering state is a good performance optimization.
374-403: Good use of useCallback for submitSearch function.Using useCallback for the submitSearch function is a good optimization that prevents unnecessary re-renders.
605-612: Good optimization of resetSearch.Using useCallback for resetSearch is a good performance optimization.
627-628: Clean integration of new UI elements.The new suggestion dropdown and date picker components are cleanly integrated into the existing UI.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/mail/components/mail/search-bar.tsx (4)
50-70: Clear dispatcher for different filter prefix cases.
Using dedicated branches for 'from', 'to', 'after', 'before', and fallback logic simplifies handling. Depending on future growth, consider grouping related conditionals or migrating to a small map/dictionary-based dispatcher if the cases expand further.
120-143: Comprehensive built-in filter set.
Defining all suggestions in a single data structure is advantageous for maintainability. As the filter set grows, watch out for repeated icons or descriptions—factor out common patterns if needed.
250-312: Text measurement for positioning can affect performance.
Creating a hidden<span>for size measurement is practical for accurate UI positioning, but doing so on every change event may be expensive on slower devices. If needed, consider deferring or throttling this calculation.
533-601: Accessible suggestions rendering.
Usingrole="listbox",role="option", andaria-selectedeffectively supports screen readers. If further refinement is needed, consider adding keyboard focus management witharia-activedescendant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/mail/search-bar.tsx(5 hunks)
🔇 Additional comments (8)
apps/mail/components/mail/search-bar.tsx (8)
27-41: Good modular approach for dynamic columns.
ThegetFilterSuggestionGridColumnsfunction is well-structured and easy to read. It adapts cleanly to different device types and suggestion counts, serving as a neat utility for dynamic grid layout.
43-48: Regex-based prefix matching looks correct.
ThematchFilterPrefixfunction accurately captures known filter prefixes and the subsequent text. Returning a tuple is straightforward for downstream usage.
72-98: Well-structured email-specific filter handling.
The code neatly handles variations between "query === 'me'" and non-empty queries, returning consistent and relevant suggestion objects. This extensible pattern makes it easy to add more conditions later (e.g., domain name checks).
100-116: Efficient prefix and query filtering.
The functionfilterByPrefixAndQueryneatly extracts the relevant filter value and reliably checks for a substring match. This approach is concise and straightforward.
215-248: State initialization is well-organized.
Introducing separate states for suggestions, date picker, and form data clarifies responsibilities and prevents collisions. Keep monitoring the complexity of local component state to ensure readability as features expand.
314-378: Keyboard navigation overrides typical text cursor movement.
Using arrow keys and Tab for suggestion interaction is a familiar pattern. Just confirm that end-users don’t need standard cursor movement within the search input at this stage, as these events are prevented. Otherwise, consider a condition (e.g., only override if suggestions are displayed).
405-434: Robust search assembly with multiple filters.
String manipulation and advanced filter insertion are well-handled insubmitSearch. If the search logic grows larger or more complex, consider a separate helper or parser for better maintainability.
603-629: DatePicker integration is intuitive.
Dynamically positioning the date picker near the cursor for 'after'/'before' filters enhances usability. The effect to hide it when clicking outside avoids accidental overlap. No issues found.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (15)
apps/mail/lib/utils.ts (2)
25-25: Consider separating folder names and labels.The array contains both folder names (e.g., "spam", "trash") and label-like items (e.g., "unread", "important"). Mixing them may lead to confusion. Consider splitting this into two distinct arrays or clearly documenting that it contains both.
92-99: Trim the extracted value.Currently, the function might return leading or trailing whitespace if the filter colon is followed by a space. Consider trimming the substring to avoid unexpected spacing issues.
- return value || ''; + return (value || '').trim();apps/mail/lib/filter.ts (3)
95-111: Return logic could be simplified.The approach is clear, but the multiple condition checks return discrete string literals. For readability, consider using a single return statement with if-else blocks or a switch. Otherwise, this is acceptable.
113-120: Optional chain suggestion from static analysis.Changing the
if (match && match[1])check to optional chaining would shorten the code slightly. This is optional, but it prevents any potential TypeError.- if (match && match[1]) { + if (match?.[1]) { return [match[0], match[1], match[2] || '']; }🧰 Tools
🪛 Biome (1.9.4)
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
172-188: Well-structured query filtering.Trimming or normalizing the filter substring could be beneficial, but this is already good for typical use cases.
apps/mail/components/mail/search-bar.tsx (10)
99-106: Use a typed state or reducer for suggestions.Maintaining multiple fields (show, filtered, activeIndex, activePrefix) is valid, but consider creating a dedicated type or using a reducer for better clarity and maintenance.
107-111: Date picker state management.Storing
show,filterType, andpositionis straightforward. A typed approach (interface) or separate reducer could further enhance readability.
135-195: Comprehensive input change handling.The logic correctly identifies prefixes, sets up suggestions, and toggles the date picker. Minor improvement: consider splitting out the date picker positioning code into a small helper function to reduce complexity.
199-267: Keyboard navigation is well-managed.Tab and arrow key handling is solid. For improved maintainability, consider extracting arrow navigation logic into a helper function or separate callback.
270-280: Outside click to close suggestions.Good approach. Might unify with the date picker’s similar effect to reduce duplicate logic if feasible.
290-319: Search term construction is flexible.Lowercasing addresses is helpful, but consider an option to preserve original casing for display or advanced cases.
321-352: Partial insertion for suggestions.The logic for rewriting the search input upon suggestion click is solid. Consider trimming extra spaces to avoid possible double spaces.
- const newValue = inputValue.substring(0, startPos) + suggestion + " " + textAfterCursor; + const newValue = inputValue.substring(0, startPos).trimEnd() + " " + suggestion + " " + textAfterCursor.trimStart();
354-403: Date filter handling.Combining both
after:andbefore:strings is logical. If future expansions are needed (e.g., supporting multiple date ranges), consider abstracting to a utility.
404-416: Click outside date picker closure.Same outside-click approach as for suggestions. DRY up the code if feasible.
496-522: Date picker popover logic.Rendering the date picker in a separate function is clean. Minor duplication with the
DateFilterapproach, but acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/mail/components/mail/search-bar.tsx(6 hunks)apps/mail/lib/filter.ts(1 hunks)apps/mail/lib/utils.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/mail/lib/filter.ts
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (13)
apps/mail/lib/utils.ts (2)
83-89: Exported type is well-structured.Defining a dedicated type for filter suggestions is clear and maintains consistency across the codebase.
101-101: Default pagination value.Establishing a default page size of 20 is a practical choice.
apps/mail/lib/filter.ts (3)
5-93: Centralized filter suggestions looks good.Hard-coding these suggestion definitions in a single array is maintainable, but consider a future approach for localization or data-driven configuration if you'll support multiple languages or dynamic additions.
122-142: Good separation of filter logic.This function nicely delegates to specialized handlers or simple filters depending on prefix and query. No major issues noted.
144-170: Email filter handling is clear.The conditional logic covering “me” and partial email queries is well-structured. Consider validating queries further (e.g., checking for valid email formats) if needed.
apps/mail/components/mail/search-bar.tsx (8)
9-9: Import looks fine.Adding the
Xicon for clearing the search is a straightforward improvement.
16-16: Extended React hooks import.No issues here—using
useState,useEffect, and others is standard for this component’s complexity.
21-21: New imports for extracted utilities.Pulling in
extractFilterValue,FilterSuggestion, andFOLDER_NAMESpromotes reusability and avoids duplication.
22-28: Additional imports for filter logic.Importing
matchFilterPrefix,filterSuggestionsFunction, andfilterSuggestionsaligns with the new advanced search approach.
418-494: Suggestion list UI is well structured.ARIA attributes are used properly for accessibility. The grid layout with dynamic columns is a nice approach for responsive design.
532-539: Reset logic.Straightforward reset that clears both internal state and external search values.
548-566: Clear search button usage.Providing a compact button with an icon is user-friendly. The accessibility label "Clear search" is a welcome detail.
593-614: Quick filters functionality.These predefined click actions are a neat convenience feature, reinforcing a straightforward user experience.
Summary by CodeRabbit
New Features
Enhancements