Conversation
…, and some fixes in sidebar, toc and ocumentation page
|
This one is ready for review |
thomasfr
left a comment
There was a problem hiding this comment.
Awesome job with the fuzzy search algo and general approach of being able to create a search index per version!!
app/routes/search.ts
Outdated
| try { | ||
| const { allPages } = await loadContentCollections(version) | ||
| const searchIndex = createSearchIndex(allPages) |
There was a problem hiding this comment.
This should be abstracted into a single "search" API and it should be called on app startup and not on every single search
There was a problem hiding this comment.
created server/search-index.ts file with that
app/routes/search.ts
Outdated
| const { allPages } = await loadContentCollections(version) | ||
| const searchIndex = createSearchIndex(allPages) | ||
|
|
||
| const results = fuzzySearch(searchIndex, query.trim()) |
There was a problem hiding this comment.
This API should not have a mandatory searchIndex param, instead there should be an API of the "search" API exported which is expecting a single object parameter with search and version properties. The "search" API itself should then have all the logic for using the correct index - which should be either already be created on app startup or at least lazily initialized per version.
There was a problem hiding this comment.
created this function, so it loads search index per version:
export async function fuzzySearch({ query, version }: CommandKSearchParams) {
const searchIndex = await getSearchIndex(version)
return useFuzzySearch(searchIndex, query)
}
There was a problem hiding this comment.
Is this really just "command-k" specific?
Also, why is the schema in a different folder and separate folder? It makes it really hard to make sense of this code. I would prefer to have code co-located in the same file as those two things are highly cohesive anyway
There was a problem hiding this comment.
Maybe some additional thoughts on this, as it's generally important: Separation of concerns is crucial. However, in this code, the CONCERN to SEPARATE is not SCHEMA and LOGIC. Instead, both are a unit and likely change together when one or the other changes. Therefore, they are cohesive and should be kept in close proximity.
In this example, I would first place the schema in the same file. If you later write tests, you can export the schema from that file and use it accordingly. As the logic in this file expands and more helper functions are added, it may be time to split the file. However, generally, having numerous small files scattered throughout the project does not provide any benefits. In fact, it complicates understanding the code, as someone would need to open multiple files and navigate back and forth. Also the "UNIT" is not a unit anymore.
Read more about it here:
https://kula.blog/posts/proximity_principle/
https://www.geeksforgeeks.org/software-engineering/software-engineering-coupling-and-cohesion/
There was a problem hiding this comment.
Is this really just "command-k" specific? Also, why is the schema in a different folder and separate folder? It makes it really hard to make sense of this code. I would prefer to have code co-located in the same file as those two things are highly cohesive anyway
i created generic parse-search-params now and I use it from the loader, where I send the schema of the params that are expected for command K search
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive Command K search component with server-side search functionality, search history management, and keyboard navigation for documentation. The search feature allows users to quickly find content across documentation pages with fuzzy search capabilities and maintains version-specific search history.
- Adds complete Command K search implementation with modal interface, search input, results display, and history management
- Implements server-side fuzzy search with search index generation from MDX content
- Adds extensive localization support for search-related UI text across multiple languages
Reviewed Changes
Copilot reviewed 34 out of 40 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| resources/locales/en/common.json | Adds English translations for search UI text, controls, and placeholders |
| resources/locales/bs/common.json | Adds Bosnian translations for search UI text, controls, and placeholders |
| content-collections.ts | Minor formatting improvement with blank line addition |
| app/utils/parse-search-params.ts | New utility for parsing and validating URL search parameters using Zod schemas |
| app/utils/local-storage.ts | Adds localStorage removal function and command-k search history key constant |
| app/utils/get-page-slug.tsx | New utility for extracting page slugs from Page objects |
| app/ui/kbd.tsx | New keyboard key display component for UI hints |
| app/ui/icon/icons/types.ts | Adds new icon names for search functionality (Trash2, Search, Pilcrow, Hash, Clock) |
| app/ui/breadcrumbs.tsx | Updates BreadcrumbItem to accept className prop for styling flexibility |
| app/tailwind.css | Adds extensive CSS custom properties for search modal theming in light and dark modes |
| app/server/search-index.ts | Server-side search functionality with index preloading and fuzzy search implementation |
| app/routes/search.ts | Search API route handler that processes search requests and returns results |
| app/routes/documentation-layout.tsx | Integrates CommandK component into the documentation layout header |
| app/routes.ts | Adds search route configuration |
| app/hooks/use-scroll-lock.ts | Custom hook for preventing body scroll when modal is open |
| app/entry.server.tsx | Preloads search indexes during server startup |
| app/components/modal.tsx | Reusable modal component with focus management and keyboard handling |
| app/components/command-k/search-types.ts | TypeScript definitions for search-related data structures |
| app/components/command-k/hooks/use-search.ts | Hook for managing search state and API calls |
| app/components/command-k/hooks/use-search-history.ts | Hook for managing version-specific search history in localStorage |
| app/components/command-k/hooks/use-modal-state.ts | Hook for managing modal open/close state |
| app/components/command-k/hooks/use-keyboard-navigation.ts | Hook for handling keyboard navigation within search results |
| app/components/command-k/hooks/use-fuzzy-search.ts | Client-side fuzzy search logic with text highlighting |
| app/components/command-k/create-search-index.ts | Utility for creating searchable index from MDX page content |
| app/components/command-k/components/trigger-button.tsx | Search trigger button with keyboard shortcuts display |
| app/components/command-k/components/search-result.tsx | Individual search result row component with icons and metadata |
| app/components/command-k/components/search-input.tsx | Search input field with icon and keyboard hints |
| app/components/command-k/components/search-history.tsx | Search history display with clear and remove functionality |
| app/components/command-k/components/results-footer.tsx | Footer showing result count and keyboard navigation hints |
| app/components/command-k/components/results-footer-note.tsx | Attribution note in search results footer |
| app/components/command-k/components/keyboard-hint.tsx | Component for displaying keyboard shortcut hints |
| app/components/command-k/components/empty-state.tsx | Empty state display for no results or initial state |
| app/components/command-k/components/command-k.tsx | Main CommandK component orchestrating all search functionality |
| app/components/backdrop.tsx | Modal backdrop component with click-to-close functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Description
id – the full slug of the item, including the #... part for navigation to a specific subtitle on a page
title – the title of the entire page (the .mdx document)
subtitle – the text of the current subtitle within that page
paragraphs – an array of strings storing the paragraphs under that subtitle
recording-2025-09-04-16-14-48.webm