feat: add SearchDialog component #13
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several enhancements and additions. A new image handling configuration is added to the Next.js config, allowing remote images from a specified host. Two new dependencies are added to the project. A new search dialog feature is integrated into the main navigation, with its implementation separated into its own component, featuring debounced input and API-based media search. In addition, new command and dialog UI components are created using Radix UI and cmdk libraries. Lastly, a custom debouncing hook is introduced to manage delayed state updates. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant MN as MainNav
participant SD as SearchDialog
participant DB as useDebounce Hook
participant API as Media API
U->>MN: Clicks search icon
MN->>SD: Opens SearchDialog
U->>SD: Types search query
SD->>DB: Sends query for debouncing
DB-->>SD: Returns debounced query
SD->>API: Sends debounced query
API-->>SD: Returns search results
SD->>U: Displays search results
U->>SD: Selects a result
SD->>MN: Triggers navigation via Next.js Router
✨ Finishing Touches
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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
next.config.ts(1 hunks)package.json(1 hunks)src/components/main-nav.tsx(2 hunks)src/components/search-dialog.tsx(1 hunks)src/components/ui/command.tsx(1 hunks)src/components/ui/dialog.tsx(1 hunks)src/hooks/use-debounce.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/search-dialog.tsx
[error] 184-184: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 216-216: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
🪛 ESLint
src/components/search-dialog.tsx
[error] 211-211: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/components/ui/command.tsx
[error] 42-42: Unknown property 'cmdk-input-wrapper' found
(react/no-unknown-property)
🔇 Additional comments (6)
next.config.ts (1)
16-23:✅ Verification successful
LGTM! Configuration for TMDB images looks correct.
The remote pattern configuration is properly set up to allow images from The Movie Database (TMDB).
Let's verify the TMDB API endpoint availability:
🏁 Script executed:
#!/bin/bash # Description: Verify TMDB image endpoint availability curl -I https://image.tmdb.org/t/p/original/example.jpgLength of output: 965
TMDB Image Configuration Approved – Note on Test URL
LGTM! The remote patterns configuration for
image.tmdb.orginnext.config.tsis correctly set up as per Next.js guidelines.Please note that the verification script returned an HTTP/2 400 status. This is most likely due to using a placeholder URL (
example.jpg), which isn’t a valid image resource on TMDB. When tested with a proper image path, the configuration should work as expected.src/components/main-nav.tsx (1)
42-44: LGTM! SearchDialog integration looks clean.The SearchDialog is properly integrated into the navigation with appropriate spacing and responsive behavior.
src/hooks/use-debounce.ts (1)
3-13: LGTM! Clean and efficient debounce implementation.The hook correctly implements debouncing with proper cleanup and type safety.
src/components/ui/dialog.tsx (1)
1-122: LGTM! Excellent dialog implementation with Radix UI.The dialog components are well-implemented with:
- Strong type safety using forwardRef
- Proper accessibility features (aria labels, keyboard navigation)
- Smooth animations with proper state transitions
- Clean component organization
src/components/ui/command.tsx (1)
1-153: LGTM! Well-structured command interface implementation.The component architecture is clean and follows React best practices:
- Good use of React.forwardRef for component composition
- Proper type definitions and prop handling
- Consistent styling approach using the cn utility
🧰 Tools
🪛 ESLint
[error] 42-42: Unknown property 'cmdk-input-wrapper' found
(react/no-unknown-property)
package.json (1)
21-21:❓ Verification inconclusive
Verify React version compatibility.
The project uses React 19.0.0, which seems to be an incorrect version number (latest stable is 18.x.x). Please verify the React version and ensure compatibility with the new dependencies.
Also applies to: 28-28, 35-35
🌐 Web query:
What is the latest stable version of React?💡 Result:
The latest stable version of React is 19.0, released on December 5, 2024[1][3][4][5][8][9]. This major update introduced features like Actions, new hooks (
useActionState,useFormStatus,useOptimistic), Server Components/Server Actions, improved Context API support, and enhanced hydration mechanisms[3][5].Key details about React v19:
- Follows Semantic Versioning principles[1]
- Supported release channel: Latest[1]
- Active support includes non-critical bug fixes and new features[1]
- Security fixes backported to all minor releases of current major versions[1]
For production applications using older versions:
- React 18.x remains under security support (latest patch: 18.3.1)[1][5]
- Earlier major versions (17.x and below) only receive critical security patches[1]
Developers can upgrade using npm/yarn with
react@19.0.0andreact-dom@19.0.0[8][9]. The official documentation at react.dev covers v19 features[6].Citations:
- 1: https://endoflife.date/react
- 2: https://reactnative.dev/versions
- 3: https://en.wikipedia.org/wiki/React_(software)
- 4: https://react.dev/blog/2024/12/05/react-19
- 5: https://github.com/facebook/react/releases
- 6: https://react.dev/versions
- 7: https://legacy.reactjs.org/versions/
- 8: https://www.npmjs.com/package/react?activeTab=versions
- 9: https://www.npmjs.com/package/react
React 19 Compatibility Assurance and Dependency Verification
Recent information confirms that React v19.0.0 is the latest stable release, so the current React version used in the project is correct. However, please verify that the newly added dependencies (e.g., @radix-ui/react-dialog@1.1.6 and cmdk@1.0.4) are fully compatible with React 19.0.0. This check should also cover the similar dependency updates noted on lines 28 and 35 in package.json.
| // export function useDebounce<T>(value: T, delay: number): T { | ||
| // const [debouncedValue, setDebouncedValue] = React.useState<T>(value) | ||
| // const timeoutRef = React.useRef<NodeJS.Timeout>(undefined) | ||
| // const previousValue = React.useRef<T>(value) | ||
|
|
||
| // React.useEffect(() => { | ||
| // // Only update if the value has changed | ||
| // if (value !== previousValue.current) { | ||
| // console.log('Value changed:', { previous: previousValue.current, current: value }) | ||
|
|
||
| // // Clear any existing timeout | ||
| // if (timeoutRef.current) { | ||
| // console.log('Clearing previous timeout') | ||
| // clearTimeout(timeoutRef.current) | ||
| // } | ||
|
|
||
| // console.log(`Setting new timeout for ${delay}ms`) | ||
| // timeoutRef.current = setTimeout(() => { | ||
| // console.log('Timeout executed, updating debounced value:', value) | ||
| // setDebouncedValue(value) | ||
| // }, delay) | ||
|
|
||
| // previousValue.current = value | ||
| // } | ||
|
|
||
| // // Cleanup on unmount or delay change | ||
| // return () => { | ||
| // if (timeoutRef.current) { | ||
| // console.log('Cleaning up timeout on unmount/update') | ||
| // clearTimeout(timeoutRef.current) | ||
| // } | ||
| // } | ||
| // }, [value, delay]) | ||
|
|
||
| // return debouncedValue | ||
| // } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove commented code.
The old implementation with logging is no longer needed and should be removed to maintain code cleanliness.
-// export function useDebounce<T>(value: T, delay: number): T {
-// const [debouncedValue, setDebouncedValue] = React.useState<T>(value)
-// const timeoutRef = React.useRef<NodeJS.Timeout>(undefined)
-// const previousValue = React.useRef<T>(value)
-
-// React.useEffect(() => {
-// // Only update if the value has changed
-// if (value !== previousValue.current) {
-// console.log('Value changed:', { previous: previousValue.current, current: value })
-
-// // Clear any existing timeout
-// if (timeoutRef.current) {
-// console.log('Clearing previous timeout')
-// clearTimeout(timeoutRef.current)
-// }
-
-// console.log(`Setting new timeout for ${delay}ms`)
-// timeoutRef.current = setTimeout(() => {
-// console.log('Timeout executed, updating debounced value:', value)
-// setDebouncedValue(value)
-// }, delay)
-
-// previousValue.current = value
-// }
-
-// // Cleanup on unmount or delay change
-// return () => {
-// if (timeoutRef.current) {
-// console.log('Cleaning up timeout on unmount/update')
-// clearTimeout(timeoutRef.current)
-// }
-// }
-// }, [value, delay])
-
-// return debouncedValue
-// }📝 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.
| // export function useDebounce<T>(value: T, delay: number): T { | |
| // const [debouncedValue, setDebouncedValue] = React.useState<T>(value) | |
| // const timeoutRef = React.useRef<NodeJS.Timeout>(undefined) | |
| // const previousValue = React.useRef<T>(value) | |
| // React.useEffect(() => { | |
| // // Only update if the value has changed | |
| // if (value !== previousValue.current) { | |
| // console.log('Value changed:', { previous: previousValue.current, current: value }) | |
| // // Clear any existing timeout | |
| // if (timeoutRef.current) { | |
| // console.log('Clearing previous timeout') | |
| // clearTimeout(timeoutRef.current) | |
| // } | |
| // console.log(`Setting new timeout for ${delay}ms`) | |
| // timeoutRef.current = setTimeout(() => { | |
| // console.log('Timeout executed, updating debounced value:', value) | |
| // setDebouncedValue(value) | |
| // }, delay) | |
| // previousValue.current = value | |
| // } | |
| // // Cleanup on unmount or delay change | |
| // return () => { | |
| // if (timeoutRef.current) { | |
| // console.log('Cleaning up timeout on unmount/update') | |
| // clearTimeout(timeoutRef.current) | |
| // } | |
| // } | |
| // }, [value, delay]) | |
| // return debouncedValue | |
| // } |
| React.ElementRef<typeof CommandPrimitive.Input>, | ||
| React.ComponentPropsWithoutRef<typeof CommandPrimitive.Input> | ||
| >(({ className, ...props }, ref) => ( | ||
| <div className="flex items-center border-b px-3" cmdk-input-wrapper=""> |
There was a problem hiding this comment.
Fix the custom attribute syntax.
The cmdk-input-wrapper attribute should use data- prefix for custom attributes to comply with HTML standards.
- <div className="flex items-center border-b px-3" cmdk-input-wrapper="">
+ <div className="flex items-center border-b px-3" data-cmdk-input-wrapper="">📝 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.
| <div className="flex items-center border-b px-3" cmdk-input-wrapper=""> | |
| <div className="flex items-center border-b px-3" data-cmdk-input-wrapper=""> |
🧰 Tools
🪛 ESLint
[error] 42-42: Unknown property 'cmdk-input-wrapper' found
(react/no-unknown-property)
| title: result.title, | ||
| media_type: '', | ||
| path: result.path, | ||
| excerpt: result.content.substring(0, 100) + '...', |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use template literals instead of string concatenation.
Replace string concatenation with template literals for better readability.
- excerpt: result.content.substring(0, 100) + '...',
+ excerpt: `${result.content.substring(0, 100)}...`,
- ? item.overview.substring(0, 100) + '...'
+ ? `${item.overview.substring(0, 100)}...`Also applies to: 216-216
🧰 Tools
🪛 Biome (1.9.4)
[error] 184-184: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| if (!response.ok) throw new Error('Search failed') | ||
|
|
||
| const data = await response.json() | ||
| console.log(`Got search results for "${debouncedQuery}":`, data) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log statements.
Remove debug console.log statements before deploying to production.
- console.log(`Got search results for "${debouncedQuery}":`, data)
- console.log(`Set search results for "${debouncedQuery}":`, [...componentResults, ...mediaResults])Also applies to: 224-224
| setResults(componentResults) | ||
|
|
||
| // Then fetch media results | ||
| const response = await fetch(`/api/search/multi?query=${encodeURIComponent(debouncedQuery)}`) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance URL encoding security.
While encodeURIComponent is used, consider additional validation of the query parameter.
- const response = await fetch(`/api/search/multi?query=${encodeURIComponent(debouncedQuery)}`)
+ const sanitizedQuery = debouncedQuery.trim().replace(/[^\w\s-]/g, '')
+ const response = await fetch(`/api/search/multi?query=${encodeURIComponent(sanitizedQuery)}`)📝 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.
| const response = await fetch(`/api/search/multi?query=${encodeURIComponent(debouncedQuery)}`) | |
| const sanitizedQuery = debouncedQuery.trim().replace(/[^\w\s-]/g, '') | |
| const response = await fetch(`/api/search/multi?query=${encodeURIComponent(sanitizedQuery)}`) |
| const mediaResults = data.results.map((item: any) => ({ | ||
| title: item.title || item.name || '', | ||
| media_type: mapMediaType(item.media_type), | ||
| path: `/${item.media_type}/${item.id}`, | ||
| excerpt: item.overview | ||
| ? item.overview.substring(0, 100) + '...' | ||
| : 'No description available', | ||
| type: 'media' as const, | ||
| posterPath: item.poster_path | ||
| ? `https://image.tmdb.org/t/p/w92${item.poster_path}` | ||
| : undefined | ||
| })) |
There was a problem hiding this comment.
Replace any type with a proper interface.
Using any type reduces type safety. Define a proper interface for the API response.
- const mediaResults = data.results.map((item: any) => ({
+ interface SearchApiResult {
+ id: number
+ title?: string
+ name?: string
+ media_type: string
+ overview?: string
+ poster_path?: string
+ }
+ const mediaResults = data.results.map((item: SearchApiResult) => ({📝 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.
| const mediaResults = data.results.map((item: any) => ({ | |
| title: item.title || item.name || '', | |
| media_type: mapMediaType(item.media_type), | |
| path: `/${item.media_type}/${item.id}`, | |
| excerpt: item.overview | |
| ? item.overview.substring(0, 100) + '...' | |
| : 'No description available', | |
| type: 'media' as const, | |
| posterPath: item.poster_path | |
| ? `https://image.tmdb.org/t/p/w92${item.poster_path}` | |
| : undefined | |
| })) | |
| interface SearchApiResult { | |
| id: number | |
| title?: string | |
| name?: string | |
| media_type: string | |
| overview?: string | |
| poster_path?: string | |
| } | |
| const mediaResults = data.results.map((item: SearchApiResult) => ({ | |
| title: item.title || item.name || '', | |
| media_type: mapMediaType(item.media_type), | |
| path: `/${item.media_type}/${item.id}`, | |
| excerpt: item.overview | |
| ? item.overview.substring(0, 100) + '...' | |
| : 'No description available', | |
| type: 'media' as const, | |
| posterPath: item.poster_path | |
| ? `https://image.tmdb.org/t/p/w92${item.poster_path}` | |
| : undefined | |
| })) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 216-216: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
🪛 ESLint
[error] 211-211: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
Summary by CodeRabbit