feat: add command palette and dialog components#3
Conversation
- Implemented Command, CommandDialog, CommandInput, CommandList, CommandEmpty, CommandGroup, CommandSeparator, CommandItem, and CommandShortcut components for a command palette UI. - Created Dialog, DialogTrigger, DialogPortal, DialogClose, DialogOverlay, DialogContent, DialogHeader, DialogFooter, DialogTitle, and DialogDescription components for a dialog UI. - Added searchCoins function to fetch coin search results from the API. - Updated package.json and package-lock.json to include new dependencies: cmdk, react-use, and swr. - Modified SearchCoin interface to make data and price_change_percentage_24h optional.
📝 WalkthroughWalkthroughAdds a coin search modal with trending and debounced search, wires it into the header, and adds exchange listings plus a safer coin-detail network derivation. Supporting UI primitives, API helpers, styling, types, and dependencies are updated. ChangesSearch Modal Feature
Exchange Listings and Coin Page Fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/coins/[id]/page.tsxOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON type.d.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/SearchModal.tsx (1)
44-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
getTrendingCoinstolib/coingecko.actions.tsfor consistency.
getTrendingCoinsis an API helper that callsfetcherfrom a'use server'module, yet it's defined and exported from this'use client'component file.searchCoinslives inlib/coingecko.actions.ts— both functions follow the same pattern and should coexist there. This improves reusability and keeps API logic centralized.♻️ Proposed refactor
Move
getTrendingCoinstolib/coingecko.actions.ts:+export async function getTrendingCoins(): Promise<TrendingCoin[]> { + const res = await fetcher<{ coins: TrendingCoin[] }>( + '/search/trending', + undefined, + 300, + ); + return res.coins; +}Then in
SearchModal.tsx, import it alongsidesearchCoins:-import { fetcher, searchCoins } from '`@/lib/coingecko.actions`'; +import { searchCoins, getTrendingCoins } from '`@/lib/coingecko.actions`';And remove the local definition and the
fetcherimport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/SearchModal.tsx` around lines 44 - 52, Move getTrendingCoins out of SearchModal and into lib/coingecko.actions.ts so it lives alongside searchCoins and the other server-side API helpers. Update SearchModal.tsx to import getTrendingCoins from that module, remove the local function definition, and drop any now-unused fetcher import so the client component only consumes the action.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/LiveDataWrapper.tsx`:
- Around line 65-70: The cell renderer in LiveDataWrapper.tsx is rendering the
exchange name as plain text and leaving Link self-closing, which creates an
empty anchor. Update the cell logic so the exchange name from
exchange.market.name is rendered as the child of Link, using exchange.trade_url
as the destination, and keep the fallback text only when the name is missing.
This should be fixed in the cell renderer inside LiveDataWrapper’s column
definition so the displayed exchange label becomes clickable.
In `@components/SearchModal.tsx`:
- Around line 77-85: Handle SWR failures explicitly in SearchModal’s useSWR
calls by destructuring error for both the search and trending-coins requests,
instead of relying on an empty array fallback alone. Update the derived state
and render logic so isNoResults only applies when there is no error, and show an
error message/state when searchCoins (and the trending fetch) fails. Use the
existing SearchModal render conditions and the useSWR hooks as the main places
to wire this in.
- Around line 24-43: Disable cmdk’s client-side filtering for the server-driven
search flow in SearchModal so results are not hidden by mismatched CommandItem
values. Update CommandDialog to accept and forward a shouldFilter prop, and set
shouldFilter to false when rendering SearchModal; keep SearchItem using coin.id
as its value/onSelect target, but ensure cmdk no longer filters those items
against the typed query.
---
Nitpick comments:
In `@components/SearchModal.tsx`:
- Around line 44-52: Move getTrendingCoins out of SearchModal and into
lib/coingecko.actions.ts so it lives alongside searchCoins and the other
server-side API helpers. Update SearchModal.tsx to import getTrendingCoins from
that module, remove the local function definition, and drop any now-unused
fetcher import so the client component only consumes the action.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e3fe5b53-9292-4c79-8115-cc1b5cd4765a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
app/coins/[id]/page.tsxapp/globals.csscomponents/LiveDataWrapper.tsxcomponents/SearchModal.tsxcomponents/header.tsxcomponents/ui/command.tsxcomponents/ui/dialog.tsxlib/coingecko.actions.tspackage.jsontype.d.ts
…al optional; comment out hasMorePages in Pagination interface
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
type.d.ts (1)
293-293: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out
hasMorePagesinstead of leaving dead code.The downstream consumer
CoinsPaginationonly usescurrentPageandtotalPages, sohasMorePagesis confirmed unused. Commented-out code clutters the interface definition — remove the line entirely.🧹 Proposed fix
interface Pagination { currentPage: number; totalPages: number; - // hasMorePages: boolean; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@type.d.ts` at line 293, Remove the commented-out hasMorePages field from the pagination type definition in the type.d.ts interface instead of leaving dead code behind. Keep the interface focused on the actually used properties, and ensure CoinsPagination continues to rely only on currentPage and totalPages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/coins/`[id]/page.tsx:
- Line 25: Remove the დარჩ debug logging from the coin page and ensure
`app/coins/[id]/page.tsx` no longer calls `console.log(coinOHLCData)` in the
page render flow. Update the `Page` component (or the relevant data-fetching
path that builds `coinOHLCData`) so it simply returns the page data without
writing the OHLC payload to the server console.
---
Nitpick comments:
In `@type.d.ts`:
- Line 293: Remove the commented-out hasMorePages field from the pagination type
definition in the type.d.ts interface instead of leaving dead code behind. Keep
the interface focused on the actually used properties, and ensure
CoinsPagination continues to rely only on currentPage and totalPages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6d4df76-5cf9-42f1-b6f5-c21e973dbfc5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
app/coins/[id]/page.tsxtype.d.ts
| }), | ||
| ]); | ||
|
|
||
| console.log(coinOHLCData); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove debug console.log before merge.
console.log(coinOHLCData) is a debug artifact that should not be shipped to production. It will log potentially large OHLC arrays to the server console on every coin page request.
🧹 Proposed fix
- console.log(coinOHLCData);📝 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.
| console.log(coinOHLCData); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/coins/`[id]/page.tsx at line 25, Remove the დარჩ debug logging from the
coin page and ensure `app/coins/[id]/page.tsx` no longer calls
`console.log(coinOHLCData)` in the page render flow. Update the `Page` component
(or the relevant data-fetching path that builds `coinOHLCData`) so it simply
returns the page data without writing the OHLC payload to the server console.
1 participant
Summary by CodeRabbit