Implement filtering and sorting, and set up drizzle client on web#36
Implement filtering and sorting, and set up drizzle client on web#36
Conversation
📝 WalkthroughWalkthroughAdds tag-based filtering and sorting to the dictionary UI, extends search hooks and backend search to accept filters/sort, integrates Drizzle ORM for local DB access, updates search schema/tokenizer and related UI components, and adds localization and UI utility components. Changes
Notes: grouped related files with ellipses and kept cohorts minimal for review focus. Sequence Diagram(s)sequenceDiagram
participant User
participant DictionaryFilters as DictionaryFilters
participant Router as Router (URL)
participant SearchLayout as SearchLayout
participant InfiniteScroll as InfiniteScroll
participant useSearch as useSearch (Hook)
participant searchDictionary as searchDictionary
participant DB as DB (Orama/Drizzle)
User->>DictionaryFilters: select tag / choose sort
DictionaryFilters->>Router: update URL search params (tags, sort)
Router->>SearchLayout: route/search params change
SearchLayout->>InfiniteScroll: pass deferred searchQuery prop
InfiniteScroll->>useSearch: request results(term, filters, sort)
useSearch->>searchDictionary: call with term, where, sortBy, offset
searchDictionary->>DB: execute search with where & sort
DB->>searchDictionary: return results
searchDictionary->>useSearch: return paginated results
useSearch->>InfiniteScroll: return data
InfiniteScroll->>User: render filtered & sorted entries
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Deploying bahar with
|
| Latest commit: |
a02a35a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://79dde2fb.bahar-5xu.pages.dev |
| Branch Preview URL: | https://feat-tags-filter-and-sorting.bahar-5xu.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/lib/db/operations/dictionary-entries.ts (1)
53-66: Fix parameter binding whensearchTermis absent.When
searchTermis falsy, the SQL omits the?placeholder but.all([searchTerm])still passes one parameter, causing a "incorrect number of bindings" error at runtime. Bind params only when the placeholder exists.🐛 Suggested fix
- const res: { tag: string; count: number }[] = await db - .prepare( - `SELECT value as tag, COUNT(*) as count - FROM dictionary_entries, json_each(tags) - WHERE value IS NOT NULL ${searchTerm ? "AND value LIKE '%' || ? || '%'" : ""} - GROUP BY value - ORDER BY count DESC;` - ) - .all([searchTerm]); + const hasSearchTerm = typeof searchTerm === "string" && searchTerm.length > 0; + const sql = `SELECT value as tag, COUNT(*) as count + FROM dictionary_entries, json_each(tags) + WHERE value IS NOT NULL ${hasSearchTerm ? "AND value LIKE '%' || ? || '%'" : ""} + GROUP BY value + ORDER BY count DESC;`; + + const res: { tag: string; count: number }[] = await db + .prepare(sql) + .all(hasSearchTerm ? [searchTerm] : []);apps/web/src/hooks/search/useSearch.ts (1)
180-198: Missing dependencies inuseEffectfor offset changes.This effect uses
sortBy,params.term,whereFilter, andsearchQueryLanguagebut only includes[offset, setHits, search]in the dependency array. This can cause stale closures where outdated filter/sort values are used when offset changes.Proposed fix
useEffect(() => { // Don't search when offset is 0 since it // was already handled in the other useEffect if (offset === 0) return; const { hits } = search( { offset, sortBy, term: params.term, where: whereFilter, }, searchQueryLanguage ); setHits((previousHits) => previousHits ? [...previousHits, ...hits] : hits ); - }, [offset, setHits, search]); + }, [offset, setHits, search, sortBy, params.term, whereFilter, searchQueryLanguage]);packages/search/src/database.ts (1)
199-204: Merging logic may conflict withsortByordering.When
sortByis provided, this merge strategy always places exact matches before fuzzy matches, which breaks the expected sort order. For example, if sorting by "recently added", a newer fuzzy match would incorrectly appear after an older exact match.Consider either:
- When
sortByis specified, skip the two-pass merge and use a single search pass- Re-sort the merged results after combining both passes
Suggested approach: single-pass when sortBy is specified
export const searchDictionary = ( db: DictionaryOrama, term: string, options?: SearchDictionaryOptions ) => { const limit = options?.limit ?? 10; const offset = options?.offset ?? 0; const language = options?.language; + const hasSortBy = !!options?.sortBy; - if (!term) { + // When sortBy is specified or no term, use single-pass search + if (!term || hasSortBy) { return search( db, { term, limit, offset, + properties: PROPERTIES, + boost: BOOST, + tolerance: term ? (stripArabicDiacritics(term).length <= 2 ? 0 : stripArabicDiacritics(term).length <= 4 ? 1 : 2) : undefined, where: options?.where, sortBy: options?.sortBy, }, language ); }
🤖 Fix all issues with AI agents
In `@apps/web/package.json`:
- Around line 37-43: The package.json currently includes `@tanstack/zod-adapter`
which requires zod@^3; remove the "@tanstack/zod-adapter" entry or replace it
with a version that supports Zod v4 (if one exists) so peer-dependency
resolution no longer conflicts with zod@4.0.17; alternatively, delete the
dependency and rely on native Zod v4 features (or update any imports/usages
referencing `@tanstack/zod-adapter`) to ensure code compiles with zod@4 used by
drizzle-zod/drizzle-orm.
In `@apps/web/src/components/features/dictionary/filters/DictionaryFilters.tsx`:
- Line 38: hasActiveFilters currently only checks filteredTags, so picking a
non-default sort doesn't mark filters active; update the boolean to also
consider whether the current sort differs from the component's default sort
(e.g., compute isSortActive = sort !== <defaultSortValueUsedInComponent> and set
hasActiveFilters = (filteredTags?.length ?? 0) > 0 || isSortActive), and apply
the same change to the other occurrence around the block handling lines 144-155
so both places use filteredTags, sort, and the component's default sort value.
In `@apps/web/src/components/features/dictionary/filters/TagsFilter.tsx`:
- Around line 70-89: When updating tags inside TagsFilter (the onSelect handler
that checks filteredTags and calls navigate), include the current sort search
param in the navigate search object so it isn't lost; retrieve the current sort
value the component uses (same source as DictionaryFilters, e.g., the existing
sort/filter state or search params) and add sort: currentSort (or undefined if
absent) alongside tags in both the removal and addition navigate calls,
mirroring the pattern used in DictionaryFilters to preserve both tags and sort.
In `@apps/web/src/components/search/InfiniteScroll.tsx`:
- Around line 448-456: The toggleExpanded callback uses the expandedIds set
returned from useSet but declares an empty dependency array; update the
useCallback for toggleExpanded to include expandedIds in its dependency array so
the hook correctly depends on the stable set reference (i.e., change
useCallback(..., []) to useCallback(..., [expandedIds])), ensuring the
exhaustive-deps rule is satisfied while retaining existing logic that
adds/deletes ids from expandedIds.
In `@apps/web/src/components/TagsCombobox.tsx`:
- Around line 21-52: The outer <button> in the TagsCombobox component is
wrapping an inner <button> (the X icon) which is invalid HTML; change the outer
element (the one using props disabled, tagValue, onClick and rendering the inner
<button> with <X />) to a non-button container (e.g., <span> or <div>) and
implement accessible behavior: add role="button", aria-disabled when disabled, a
tabindex (when not disabled), and onKeyDown to activate on Enter/Space and call
onClick just like the original handler; also remove the misplaced key={tagValue}
from inside this component (keys belong at the call site). Ensure
event.stopPropagation() semantics for both the outer container handlers and the
inner close button handlers remain the same.
In `@apps/web/src/hooks/search/useSearch.ts`:
- Around line 202-217: The useEffect in useSearch.ts that calls search(...) can
run with stale values because its dependency array omits sortBy, params.term,
whereFilter, and searchQueryLanguage (and setHasMore); update the dependency
array for that effect (the useEffect that sets offset, hits, metadata, and
hasMore) to include sortBy, params.term (or params), whereFilter,
searchQueryLanguage, and setHasMore in addition to the existing dependencies
(paramsKey, setOffset, setHits, setSearchResultsMetadata, search) so the effect
re-runs whenever those memoized/derived values change.
In
`@apps/web/src/routes/_authorized-layout/_app-layout/dictionary/edit/`$wordId.tsx:
- Around line 198-217: Replace the direct history.back() call with TanStack
Router's can-go-back check: import and call useCanGoBack() alongside
useRouter(), then update the Button onClick to conditionally call
router.history.back() if canGoBack is true, otherwise perform a safe in-app
navigation via router.navigate (e.g., navigate to your dictionary/listing
route). Reference: useCanGoBack(), router, router.history.back(),
router.navigate().
In `@packages/drizzle-user-db-schemas/package.json`:
- Line 13: The package.json pins the peer dependency "drizzle-orm" to a specific
pre-release ("1.0.0-beta.10-4a43a22"); change the "drizzle-orm" entry in
package.json to a stable or flexible semver range (e.g., "^1.0.0" or ">=0.23.13
<2.0.0") instead of the specific beta, save the file, and regenerate lockfiles
(npm/yarn/pnpm) so consumers aren't forced to that pre-release; update the
peerDependencies/devDependencies entries referencing "drizzle-orm" if present
and run your install to verify no conflicts.
In `@packages/web-ui/src/components/scroll-area.tsx`:
- Around line 23-43: The component ScrollBar is importing non-existent Radix
exports ScrollAreaPrimitive.ScrollAreaScrollbar and
ScrollAreaPrimitive.ScrollAreaThumb; update all uses to the correct v1.2.10
exports ScrollAreaPrimitive.Scrollbar and ScrollAreaPrimitive.Thumb (including
the JSX elements and the displayName assignment), i.e., replace references to
ScrollAreaPrimitive.ScrollAreaScrollbar with ScrollAreaPrimitive.Scrollbar and
ScrollAreaPrimitive.ScrollAreaThumb with ScrollAreaPrimitive.Thumb inside the
ScrollBar forwardRef component and when setting ScrollBar.displayName.
🧹 Nitpick comments (8)
apps/web/src/components/features/decks/DeckDialogContent.tsx (1)
174-184: Consider logging the error for debugging purposes.The underscore prefix convention for unused variables is appropriate. However, silently discarding the error makes debugging production issues difficult. Consider logging the error before showing the user-friendly toast.
💡 Suggested improvement
- } catch (_err) { + } catch (err) { + console.error("Deck operation failed:", err); if (isEditing) {packages/search/src/schema.ts (2)
36-51: Stale interface:IndexedMorphologystill contains_exactfields.The schema removed exact-variant fields (per AI summary), but
IndexedMorphologystill definessingular_exact,plurals_exact,past_tense_exact,present_tense_exact, andmasadir_exact. These should likely be removed to keep the interface consistent with the schema.Proposed cleanup
export interface IndexedMorphology { ism?: { singular?: string; plurals?: string[]; - singular_exact?: string; - plurals_exact?: string[]; }; verb?: { past_tense?: string; present_tense?: string; masadir?: string[]; - past_tense_exact?: string; - present_tense_exact?: string; - masadir_exact?: string[]; }; }
57-73: Stale interface:DictionaryDocumentstill containsword_exact.Similar to
IndexedMorphology, theword_exactfield on line 60 should be removed if exact-variant fields were intentionally removed from the schema.Proposed cleanup
export interface DictionaryDocument { id: string; word: string; - word_exact?: string; translation: string; // ... rest unchanged }apps/web/src/lib/db/index.ts (1)
175-207: Drizzle proxy implementation looks correct, but silent failure may mask issues.The sqlite-proxy driver implementation correctly handles all method types (
run,all,values,get). However, Line 177 silently returns{ rows: [] }whendbis null, which could mask initialization issues during debugging.Consider logging or throwing when this unexpected state occurs:
Optional: Add warning for unexpected null db
async (sql, params, method) => { - if (!db) return { rows: [] }; + if (!db) { + console.warn("Drizzle query attempted before db initialization"); + return { rows: [] }; + }apps/web/src/hooks/search/useSearch.ts (1)
144-162: UsingparamsKeyfor memo dependencies is acceptable but consider individual deps.Using
JSON.stringify(params)as the dependency works but has overhead. Sinceparams.filters?.tagsandparams.sortare the actual dependencies, using them directly would be more precise. However, this is a minor optimization concern.Optional: Use specific dependencies
const whereFilter = useMemo<SearchDictionaryOptions["where"]>(() => { if (!params.filters?.tags?.length) return undefined; return { tags: { containsAll: params.filters.tags }, }; - }, [paramsKey]); + }, [params.filters?.tags]); const sortBy = useMemo<SearchDictionaryOptions["sortBy"]>(() => { if (!params.sort || params.sort === "relevance") return undefined; return { property: params.sort === "createdAt" ? "created_at_timestamp_ms" : "updated_at_timestamp_ms", order: "DESC", }; - }, [paramsKey]); + }, [params.sort]);apps/web/src/components/features/dictionary/filters/DictionaryFilters.tsx (1)
17-31: Avoid duplicating sort options.Consider sourcing the UI list from the same shared constant used by the search hook so UI/options and search validation can’t drift.
apps/web/src/routes/_authorized-layout/_search-layout/index.lazy.tsx (2)
28-31: Keep header counts aligned with the rendered results.
InfiniteScrollnow usesdeferredSearchQuery, but the header counts come fromuseSearch()directly. During fast typing, the counts can reflect a newer query than the list. Consider basing both on the same query value to avoid mismatched UI.Also applies to: 53-57, 165-165
155-156: Remove the commented‑out divider.It’s dead code and adds noise to the layout. As per coding guidelines, avoid commented‑out code.
🧹 Cleanup
- {/*<div className="mx-4 h-px bg-linear-to-r from-border/50 via-border to-border/50 sm:mx-6" />*/}
| "@tanstack/zod-adapter": "^1.150.0", | ||
| "@tursodatabase/sync-wasm": "^0.3.2", | ||
| "@uidotdev/usehooks": "^2.4.1", | ||
| "better-auth": "1.4.9", | ||
| "date-fns": "^3.6.0", | ||
| "dompurify": "^3.3.0", | ||
| "drizzle-orm": "^0.45.1", | ||
| "drizzle-orm": "1.0.0-beta.10-4a43a22", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@tanstack/zod-adapter 1.150.0 compatibility requirements zod 4 @tanstack/react-router``
💡 Result:
Short answer:
@tanstack/zod-adapter@1.150.0 is intended for Zod v3 (peerDependency: zod ^3.23.8) and requires a TanStack Router / React Router version >= 1.43.2. [1][2]- If you want to use Zod v4: recent Zod releases (>= v4.0.6) added behavior that makes the adapter unnecessary for many use-cases (the repo issue notes Zod‑4 compatibility/workarounds and that Zod >= 4.0.6 works natively). If you rely on any adapter-specific helpers (e.g. fallback behavior), check the issue discussion and test with your exact versions. [2][3]
Sources:
- TanStack release listing showing
@tanstack/zod-adapter@1.150.0. [1] @tanstack/zod-adapterpackage peerDependency info / docs (zod: ^3.23.8,@tanstack/react-router: >=1.43.2). [2]- GitHub issue discussing Zod v4 support and notes about Zod >= 4.0.6. [3]
🌐 Web query:
drizzle-orm 1.0.0-beta.10-4a43a22 compatibility drizzle-zod 0.8.3 zod
💡 Result:
Short answer: Generally compatible — drizzle-zod@0.8.3 supports Zod v3 (>=3.25.1) and v4 and declares drizzle-orm >=0.36.0 as a peer, so it should work with a drizzle-orm 1.0.0-beta build. [1][2][3]
Notes / caveats:
- drizzle-zod docs state requirement: Drizzle ORM >= 0.36.0 and Zod >= 3.25.1 (with v4 supported). [2]
- npm shows drizzle-zod@0.8.3 published recently. [3]
- There are GitHub issues about peer-dependency bumps for beta drizzle-orm builds (some beta tags can cause npm peer-dep resolution warnings); if you use a specific 1.0.0-beta commit tag you may see peerDep warnings and might need to align package tags or use package-manager overrides/resolutions. [4]
If you want, tell me the exact drizzle-orm beta tag you’re using (full version string) and your package manager (npm/pnpm/yarn) and I’ll check for any known breakages or provide a concrete install/resolution command.
References:
[1] Drizzle docs — drizzle-zod page (requirements).
[2] Drizzle docs / changelog notes about zod support.
[3] npm: drizzle-zod (v0.8.3).
[4] GitHub issue re: peer dependency bumps for beta builds.
Remove or downgrade @tanstack/zod-adapter — it requires Zod v3 but the project uses Zod v4.
@tanstack/zod-adapter@1.150.0 declares a peer dependency of zod@^3.23.8 and is incompatible with zod@4.0.17. This will cause peer-dependency resolution warnings or failures. Since Zod v4 (>= 4.0.6) added native support for the adapter's functionality, consider removing the adapter entirely or using a newer adapter version if one exists for Zod v4.
The drizzle-orm@1.0.0-beta.10-4a43a22 is compatible with drizzle-zod@0.8.3 and zod@4.0.17.
🤖 Prompt for AI Agents
In `@apps/web/package.json` around lines 37 - 43, The package.json currently
includes `@tanstack/zod-adapter` which requires zod@^3; remove the
"@tanstack/zod-adapter" entry or replace it with a version that supports Zod v4
(if one exists) so peer-dependency resolution no longer conflicts with
zod@4.0.17; alternatively, delete the dependency and rely on native Zod v4
features (or update any imports/usages referencing `@tanstack/zod-adapter`) to
ensure code compiles with zod@4 used by drizzle-zod/drizzle-orm.
| const expandedIds = useSet<string>(); | ||
|
|
||
| const toggleExpanded = useCallback((id: string) => { | ||
| setExpandedIds((prev) => { | ||
| const next = new Set(prev); | ||
| if (next.has(id)) { | ||
| next.delete(id); | ||
| } else { | ||
| next.add(id); | ||
| } | ||
| return next; | ||
| }); | ||
| if (expandedIds.has(id)) { | ||
| expandedIds.delete(id); | ||
| } else { | ||
| expandedIds.add(id); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Missing dependency in useCallback for toggleExpanded.
The toggleExpanded callback references expandedIds but has an empty dependency array. While useSet returns a stable reference, the callback should include expandedIds for correctness and to satisfy the exhaustive-deps rule.
Proposed fix
const toggleExpanded = useCallback((id: string) => {
if (expandedIds.has(id)) {
expandedIds.delete(id);
} else {
expandedIds.add(id);
}
- }, []);
+ }, [expandedIds]);🤖 Prompt for AI Agents
In `@apps/web/src/components/search/InfiniteScroll.tsx` around lines 448 - 456,
The toggleExpanded callback uses the expandedIds set returned from useSet but
declares an empty dependency array; update the useCallback for toggleExpanded to
include expandedIds in its dependency array so the hook correctly depends on the
stable set reference (i.e., change useCallback(..., []) to useCallback(...,
[expandedIds])), ensuring the exhaustive-deps rule is satisfied while retaining
existing logic that adds/deletes ids from expandedIds.
| "peerDependencies": { | ||
| "zod": "4.0.17", | ||
| "drizzle-orm": "^0.45.1", | ||
| "drizzle-orm": "1.0.0-beta.10-4a43a22", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
drizzle-orm 1.0.0-beta.10-4a43a22 npm package release
💡 Result:
The npm release "drizzle-orm@1.0.0-beta.10-4a43a22" is a published beta build (part of the 1.0.0-beta.10 series) — listed on package registries with a publish date of January 9, 2026. [1][2][3]
Sources:
- Yarn package versions list (shows 1.0.0-beta.10-4a43a22, Jan 9, 2026). [1]
- NewReleases summary listing that build among recent beta releases. [2]
- Drizzle ORM releases page (context about the 1.0 beta series). [3]
Using a pre-release peer dependency for drizzle-orm introduces fragmentation risk.
The version 1.0.0-beta.10-4a43a22 exists on npm and satisfies the peer dependency requirement (drizzle-zod@0.5.1 requires >=0.23.13), but publishing a beta version locks consumers into a specific pre-release build. Consider using a stable release version to allow consumers flexibility and reduce maintenance burden.
🤖 Prompt for AI Agents
In `@packages/drizzle-user-db-schemas/package.json` at line 13, The package.json
pins the peer dependency "drizzle-orm" to a specific pre-release
("1.0.0-beta.10-4a43a22"); change the "drizzle-orm" entry in package.json to a
stable or flexible semver range (e.g., "^1.0.0" or ">=0.23.13 <2.0.0") instead
of the specific beta, save the file, and regenerate lockfiles (npm/yarn/pnpm) so
consumers aren't forced to that pre-release; update the
peerDependencies/devDependencies entries referencing "drizzle-orm" if present
and run your install to verify no conflicts.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/components/features/dictionary/filters/DictionaryFilters.tsx`:
- Around line 74-97: The TagPill onClick handler in DictionaryFilters is using
the render-time variable filteredTags, which can be stale; update the navigate
search updater to compute the new tags from prev.tags instead (inside the
search: (prev) => { ... }) and perform the toggle logic there (check if
prev.tags includes the clicked tag, filter it out or append it accordingly), so
updates are atomic and avoid lost toggles when clicking tags rapidly.
In `@apps/web/src/components/features/dictionary/filters/TagsFilter.tsx`:
- Around line 70-90: The onSelect handler for tag toggles currently reads
filteredTags from render-time closures and then calls navigate with a search
updater that uses that stale value; instead, move the add/remove logic into the
navigate search updater so you compute new tags from prev.tags at call time (use
prev.tags to determine whether to remove or add the current tag variable), then
return the updated tags (or undefined when empty). Update the handler around the
existing navigate(...) call used in TagsFilter.tsx to reference prev.tags inside
the search function rather than using filteredTags captured earlier.
♻️ Duplicate comments (1)
packages/web-ui/src/components/scroll-area.tsx (1)
23-43: Fix incorrect Radix ScrollArea primitive names.This issue has already been identified in a previous review. The code uses non-existent exports
ScrollAreaPrimitive.ScrollAreaScrollbarandScrollAreaPrimitive.ScrollAreaThumb. The correct exports in@radix-ui/react-scroll-areaareScrollbarandThumb.
🧹 Nitpick comments (1)
apps/web/src/components/features/dictionary/filters/TagsFilter.tsx (1)
30-33: Surface tag-query failures instead of silently showing the empty state.
useQueryerrors currently render the same UI as “no tags,” which can mask DB/query failures. Consider handlingisError/errorand showing aDisplayError(or an inline fallback) so failures are visible.
| <TagPill | ||
| key={tag} | ||
| onClick={() => { | ||
| const shouldRemove = filteredTags?.some((t) => t === tag); | ||
|
|
||
| if (shouldRemove) { | ||
| const newTags = filteredTags?.filter((t) => t !== tag); | ||
|
|
||
| navigate({ | ||
| to: "/", | ||
| search: (prev) => ({ | ||
| ...prev, | ||
| tags: newTags?.length ? newTags : undefined, | ||
| }), | ||
| }); | ||
| } else { | ||
| navigate({ | ||
| to: "/", | ||
| search: (prev) => ({ | ||
| ...prev, | ||
| tags: [...(filteredTags ?? []), tag], | ||
| }), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/components/features/dictionary/filters/DictionaryFilters.tsxRepository: Shunseii/bahar
Length of output: 6473
🏁 Script executed:
# Search for other uses of navigate with search updaters in the codebase
rg -A 8 'navigate\(' apps/web/src --type tsx --type ts | head -100Repository: Shunseii/bahar
Length of output: 85
🏁 Script executed:
# Search for navigate with search updater patterns in the codebase
rg -A 8 'navigate\(' apps/web/src | grep -A 8 'search:' | head -80Repository: Shunseii/bahar
Length of output: 4208
🏁 Script executed:
# Check TagsFilter.tsx to see if it has similar pattern and context
cat -n apps/web/src/components/features/dictionary/filters/TagsFilter.tsx | head -150Repository: Shunseii/bahar
Length of output: 4266
🏁 Script executed:
# Look for TanstackRouter documentation or check if there are any other patterns
# that might show how the search updater works
rg 'search:.*prev.*=>' apps/web/src -A 5 | head -60Repository: Shunseii/bahar
Length of output: 3297
Use prev.tags inside the search updater to avoid stale tag toggles.
The handler captures render-time filteredTags which can be stale if multiple tag clicks happen quickly. Computing from prev.tags ensures atomic updates.
✅ Proposed fix
- onClick={() => {
- const shouldRemove = filteredTags?.some((t) => t === tag);
-
- if (shouldRemove) {
- const newTags = filteredTags?.filter((t) => t !== tag);
-
- navigate({
- to: "/",
- search: (prev) => ({
- ...prev,
- tags: newTags?.length ? newTags : undefined,
- }),
- });
- } else {
- navigate({
- to: "/",
- search: (prev) => ({
- ...prev,
- tags: [...(filteredTags ?? []), tag],
- }),
- });
- }
- }}
+ onClick={() => {
+ navigate({
+ to: "/",
+ search: (prev) => {
+ const prevTags = prev.tags ?? [];
+ const nextTags = prevTags.includes(tag)
+ ? prevTags.filter((t) => t !== tag)
+ : [...prevTags, tag];
+
+ return {
+ ...prev,
+ tags: nextTags.length ? nextTags : undefined,
+ };
+ },
+ });
+ }}📝 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.
| <TagPill | |
| key={tag} | |
| onClick={() => { | |
| const shouldRemove = filteredTags?.some((t) => t === tag); | |
| if (shouldRemove) { | |
| const newTags = filteredTags?.filter((t) => t !== tag); | |
| navigate({ | |
| to: "/", | |
| search: (prev) => ({ | |
| ...prev, | |
| tags: newTags?.length ? newTags : undefined, | |
| }), | |
| }); | |
| } else { | |
| navigate({ | |
| to: "/", | |
| search: (prev) => ({ | |
| ...prev, | |
| tags: [...(filteredTags ?? []), tag], | |
| }), | |
| }); | |
| } | |
| <TagPill | |
| key={tag} | |
| onClick={() => { | |
| navigate({ | |
| to: "/", | |
| search: (prev) => { | |
| const prevTags = prev.tags ?? []; | |
| const nextTags = prevTags.includes(tag) | |
| ? prevTags.filter((t) => t !== tag) | |
| : [...prevTags, tag]; | |
| return { | |
| ...prev, | |
| tags: nextTags.length ? nextTags : undefined, | |
| }; | |
| }, | |
| }); | |
| }} |
🤖 Prompt for AI Agents
In `@apps/web/src/components/features/dictionary/filters/DictionaryFilters.tsx`
around lines 74 - 97, The TagPill onClick handler in DictionaryFilters is using
the render-time variable filteredTags, which can be stale; update the navigate
search updater to compute the new tags from prev.tags instead (inside the
search: (prev) => { ... }) and perform the toggle logic there (check if
prev.tags includes the clicked tag, filter it out or append it accordingly), so
updates are atomic and avoid lost toggles when clicking tags rapidly.
| onSelect={() => { | ||
| const shouldRemove = filteredTags?.some((t) => t === tag); | ||
|
|
||
| if (shouldRemove) { | ||
| const newTags = filteredTags?.filter((t) => t !== tag); | ||
|
|
||
| navigate({ | ||
| to: "/", | ||
| search: (prev) => ({ | ||
| ...prev, | ||
| tags: newTags?.length ? newTags : undefined, | ||
| }), | ||
| }); | ||
| } else { | ||
| navigate({ | ||
| to: "/", | ||
| search: (prev) => ({ | ||
| ...prev, | ||
| tags: [...(filteredTags ?? []), tag], | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/components/features/dictionary/filters/TagsFilter.tsxRepository: Shunseii/bahar
Length of output: 4266
Compute tag changes from router state in updater to avoid stale closures with rapid toggles.
The onSelect handler captures filteredTags at render time, then uses it inside the search updater. With rapid clicks, multiple navigate calls queue before the router updates, so later calls operate on stale closure values. Move the tag addition/removal logic inside the search updater function to compute from prev.tags instead.
Proposed fix
- onSelect={() => {
- const shouldRemove = filteredTags?.some((t) => t === tag);
-
- if (shouldRemove) {
- const newTags = filteredTags?.filter((t) => t !== tag);
-
- navigate({
- to: "/",
- search: (prev) => ({
- ...prev,
- tags: newTags?.length ? newTags : undefined,
- }),
- });
- } else {
- navigate({
- to: "/",
- search: (prev) => ({
- ...prev,
- tags: [...(filteredTags ?? []), tag],
- }),
- });
- }
- }}
+ onSelect={() => {
+ navigate({
+ to: "/",
+ search: (prev) => {
+ const prevTags = prev.tags ?? [];
+ const nextTags = prevTags.includes(tag)
+ ? prevTags.filter((t) => t !== tag)
+ : [...prevTags, tag];
+
+ return {
+ ...prev,
+ tags: nextTags.length ? nextTags : undefined,
+ };
+ },
+ });
+ }}📝 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.
| onSelect={() => { | |
| const shouldRemove = filteredTags?.some((t) => t === tag); | |
| if (shouldRemove) { | |
| const newTags = filteredTags?.filter((t) => t !== tag); | |
| navigate({ | |
| to: "/", | |
| search: (prev) => ({ | |
| ...prev, | |
| tags: newTags?.length ? newTags : undefined, | |
| }), | |
| }); | |
| } else { | |
| navigate({ | |
| to: "/", | |
| search: (prev) => ({ | |
| ...prev, | |
| tags: [...(filteredTags ?? []), tag], | |
| }), | |
| }); | |
| onSelect={() => { | |
| navigate({ | |
| to: "/", | |
| search: (prev) => { | |
| const prevTags = prev.tags ?? []; | |
| const nextTags = prevTags.includes(tag) | |
| ? prevTags.filter((t) => t !== tag) | |
| : [...prevTags, tag]; | |
| return { | |
| ...prev, | |
| tags: nextTags.length ? nextTags : undefined, | |
| }; | |
| }, | |
| }); | |
| }} |
🤖 Prompt for AI Agents
In `@apps/web/src/components/features/dictionary/filters/TagsFilter.tsx` around
lines 70 - 90, The onSelect handler for tag toggles currently reads filteredTags
from render-time closures and then calls navigate with a search updater that
uses that stale value; instead, move the add/remove logic into the navigate
search updater so you compute new tags from prev.tags at call time (use
prev.tags to determine whether to remove or add the current tag variable), then
return the updated tags (or undefined when empty). Update the handler around the
existing navigate(...) call used in TagsFilter.tsx to reference prev.tags inside
the search function rather than using filteredTags captured earlier.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/search/InfiniteScroll.tsx (2)
76-80: Wrap clipboard operations in try/catch to handle permission failures and unsupported contexts.
navigator.clipboard.writeTextcan reject without proper permissions or in sandboxed contexts, leading to unhandled promise rejections. Add error handling withDisplayErrorto provide user feedback on failure.Suggested update
const handleCopy = async () => { - await navigator.clipboard.writeText(text); - setCopied(true); - setTimeout(() => setCopied(false), 1500); + try { + await navigator.clipboard.writeText(text); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch (error) { + setCopied(false); + // surface a DisplayError/toast for copy failures + } };
470-474: IncludehasMoreandshowMorein the effect dependencies.The effect uses both
hasMoreandshowMorebut only depends onshouldLoadMore. IfhasMorebecomes true whileshouldLoadMorestays true, the effect won't re-run and infinite scrolling will stall. SinceshowMoreis already memoized viauseCallback, adding these dependencies won't cause infinite loops.🧩 Suggested update
- }, [shouldLoadMore]); + }, [shouldLoadMore, hasMore, showMore]);
🤖 Fix all issues with AI agents
In `@apps/web/src/components/search/InfiniteScroll.tsx`:
- Around line 54-69: The SecondaryIconButton component currently renders
icon-only buttons without accessible labels; update its props to accept an
ariaLabel: string (e.g., add ariaLabel?: string to SecondaryIconButton's props)
and set that value on the rendered Button as aria-label={ariaLabel}; then update
all callers (copy/edit/expand toggles) to pass localized labels from
Lingui—providing distinct labels for states like expanded vs collapsed—and
ensure any optional onClick usage still works when ariaLabel is provided.
| const SecondaryIconButton = ({ | ||
| onClick, | ||
| children, | ||
| }: { | ||
| onClick?: () => void; | ||
| children: ReactNode; | ||
| }) => { | ||
| return ( | ||
| <Button | ||
| className="h-8 w-8 text-muted-foreground hover:bg-primary/10 hover:text-foreground" | ||
| onClick={onClick} | ||
| size="icon" | ||
| variant="ghost" | ||
| > | ||
| {children} | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/components/search/InfiniteScroll.tsx | head -100Repository: Shunseii/bahar
Length of output: 3094
🏁 Script executed:
wc -l apps/web/src/components/search/InfiniteScroll.tsxRepository: Shunseii/bahar
Length of output: 111
🏁 Script executed:
rg "ariaLabel|aria-label|useLingui" apps/web/src/components/search/InfiniteScroll.tsxRepository: Shunseii/bahar
Length of output: 171
🏁 Script executed:
cat -n apps/web/src/components/search/InfiniteScroll.tsx | sed -n '54,90p'Repository: Shunseii/bahar
Length of output: 1137
🏁 Script executed:
cat -n apps/web/src/components/search/InfiniteScroll.tsx | sed -n '345,387p'Repository: Shunseii/bahar
Length of output: 1945
🏁 Script executed:
rg "SecondaryIconButton|<Button" apps/web/src/components/search/InfiniteScroll.tsx -A 2Repository: Shunseii/bahar
Length of output: 767
🏁 Script executed:
grep -n "clipboard\|error" apps/web/src/components/search/InfiniteScroll.tsxRepository: Shunseii/bahar
Length of output: 107
Add accessible labels for icon-only actions.
Icon-only buttons (copy/edit/expand) have no aria-label, so screen readers cannot announce the action. Add an ariaLabel prop to SecondaryIconButton and pass localized labels from callers (including expanded/collapsed state).
♿ Suggested update
-const SecondaryIconButton = ({
- onClick,
- children,
-}: {
- onClick?: () => void;
- children: ReactNode;
-}) => {
+const SecondaryIconButton = ({
+ onClick,
+ children,
+ ariaLabel,
+}: {
+ onClick?: () => void;
+ children: ReactNode;
+ ariaLabel: string;
+}) => {
return (
<Button
+ aria-label={ariaLabel}
className="h-8 w-8 text-muted-foreground hover:bg-primary/10 hover:text-foreground"
onClick={onClick}
size="icon"
variant="ghost"
>
{children}
</Button>
);
};
const CopyButton: FC<{ text: string }> = memo(({ text }) => {
+ const { t } = useLingui();
const [copied, setCopied] = useState(false);
@@
- <SecondaryIconButton onClick={handleCopy}>
+ <SecondaryIconButton onClick={handleCopy} ariaLabel={t`Copy word`}>
{copied ? (
<Check className="h-4 w-4 text-green-500" />
) : (
<Copy className="h-4 w-4" />
)}
</SecondaryIconButton>
);
});
const WordCardContent: FC<WordCardContentProps> = memo(
({ hit, isExpanded, onToggleExpanded, onNavigateEdit }) => {
+ const { t } = useLingui();
@@
- <SecondaryIconButton onClick={handleEdit}>
+ <SecondaryIconButton onClick={handleEdit} ariaLabel={t`Edit word`}>
<Edit className="h-4 w-4" />
</SecondaryIconButton>
@@
- <SecondaryIconButton onClick={handleToggle}>
+ <SecondaryIconButton
+ onClick={handleToggle}
+ ariaLabel={isExpanded ? t`Collapse details` : t`Expand details`}
+ >
<motion.div
animate={{ rotate: isExpanded ? 180 : 0 }}
transition={{ duration: 0.2 }}
>Use Lingui for user-facing strings, including aria-labels, as per coding guidelines.
🤖 Prompt for AI Agents
In `@apps/web/src/components/search/InfiniteScroll.tsx` around lines 54 - 69, The
SecondaryIconButton component currently renders icon-only buttons without
accessible labels; update its props to accept an ariaLabel: string (e.g., add
ariaLabel?: string to SecondaryIconButton's props) and set that value on the
rendered Button as aria-label={ariaLabel}; then update all callers
(copy/edit/expand toggles) to pass localized labels from Lingui—providing
distinct labels for states like expanded vs collapsed—and ensure any optional
onClick usage still works when ariaLabel is provided.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.