feat(search): sectioned top-bar results — artists, albums, titles#326
Merged
Conversation
The global search advertised titles/artists/albums but the dropdown only
listed tracks, with no way to reach an album or artist page.
Backend: add `search_albums` / `search_artists` commands in browse.rs.
FTS is track-scoped, so they substring-match the query's `canonical_name`
form against `album.canonical_title` / `artist.canonical_name` (prefix
matches rank first) and return the same slim `{ artwork_base, items }`
shape as `list_albums` / `list_artists`. The per-row thumbnail expansion
is factored into shared `expand_album_rows` / `expand_artist_rows`
helpers reused by both the list and search paths.
Frontend: `searchAlbums` / `searchArtists` wrappers; TopBar fans the
three entities out in one `Promise.all` and renders sectioned results
(Artists / Albums / Titles). Artist + album rows navigate to the detail
views via new `onNavigateToArtist` / `onNavigateToAlbum` props; title
rows still play. Advanced filters stay track-only — when any is set the
album/artist sections are suppressed.
i18n: `topbar.search.{artists,albums,titles}` across all 17 locales.
Docs: library.md Search section + CLAUDE.md catalogue.
InstaZDLL
added
scope: frontend
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLa recherche de la TopBar renvoie désormais des albums et des artistes en plus des titres. Le backend, les commandes Tauri, les wrappers TypeScript et l’UI ont été mis à jour, avec des libellés localisés et une documentation alignée. ChangesRecherche sectionnée dans la TopBar
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🤖 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 `@docs/features/library.md`:
- Line 40: The advanced filter description in the library feature docs is
partially enumerating track-only filters and is missing other conditions handled
by TopBar.tsx. Update the wording to avoid listing an incomplete set of filters;
instead describe the behavior generically as “when any advanced filter is set”
or “when track-only filters are active,” so it stays aligned with the track-only
branching in TopBar and the related search flow.
In `@src/components/layout/TopBar.tsx`:
- Around line 151-177: The search flow in TopBar should ignore stale responses,
since Promise.all(...).then(...) can apply results from an older request after a
newer query has already been issued. Update the search handler around
tracksPromise, albumsPromise, artistsPromise, and the Promise.all callback to
track the latest request with a ref-based requestId or snapshot, and only call
setSearchResults, setAlbumResults, setArtistResults, and setIsSearchOpen when
the response matches the current request.
- Around line 82-88: The focus-reopen logic in TopBar still only checks track
matches, so searches with only albums or artists can look empty on refocus.
Update the menu reopen condition used by the input/focus handling in TopBar to
consider albumResults and artistResults alongside searchResults, and make sure
the state/effect that controls the dropdown uses all three result sets
consistently.
- Around line 581-663: The search result rows in TopBar are clickable but not
keyboard-accessible because the artist, album, and track entries are rendered as
non-interactive li elements with onClick only. Update the result items inside
the artistResults, albumResults, and searchResults sections to use a semantic
interactive control such as a full-width button type="button" or link, and wire
it to handleArtistResultClick, handleAlbumResultClick, and
handleSearchResultClick so they can be activated by keyboard and announced
correctly by assistive tech.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7c41c4db-c5c3-4c28-a296-63822bab7502
📒 Files selected for processing (24)
CLAUDE.mddocs/features/library.mdsrc-tauri/crates/app/src/commands/browse.rssrc-tauri/crates/app/src/lib.rssrc/components/layout/AppLayout.tsxsrc/components/layout/TopBar.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.jsonsrc/lib/tauri/browse.ts
- Guard against stale responses: each `runSearch` bumps a `searchSeqRef`
and the `Promise.all` callback only applies results when its id is
still current, so a slow older request can't clobber a newer query.
- Focus-reopen now considers album + artist results, not just tracks —
an album-only / artist-only result set no longer looks empty on
refocus.
- Result rows are now keyboard-accessible: each artist / album / title
entry is a full-width `<button type="button">` inside its `<li>`
(Tab + Enter/Space, announced by assistive tech) instead of an
`onClick`-only `<li>`.
- Docs: describe the advanced-filter suppression generically ("when any
advanced filter is active") instead of an incomplete filter list.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/layout/TopBar.tsx (1)
246-252: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvalide la recherche en cours à la fermeture.
closeSearch()vide l’état sans incrémentersearchSeqRef. Si une recherche déjà lancée se résout après un clic sur artiste/album/titre, sonseqreste courant et peut rouvrir le dropdown après la navigation ou la lecture.Correctif proposé
const closeSearch = () => { + searchSeqRef.current += 1; setIsSearchOpen(false); setSearchQuery(""); setSearchResults([]); setAlbumResults([]); setArtistResults([]); };As per path instructions,
src/**: "Vérifie les erreurs de state/effects, les handlers qui lancent des commandes Tauri, les problèmes d'accessibilité visibles, et les régressions de navigation/player."🤖 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 `@src/components/layout/TopBar.tsx` around lines 246 - 252, La fermeture de la recherche ne invalide pas les requêtes encore en vol, ce qui peut laisser une réponse ancienne réouvrir l’UI après navigation. Dans closeSearch(), incrémentez searchSeqRef en plus de vider les états, afin que les résultats déjà lancés deviennent obsolètes; gardez la logique de recherche/filtrage dans les handlers associés à searchSeqRef pour rejeter toute réponse avec un seq périmé.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/components/layout/TopBar.tsx`:
- Around line 246-252: La fermeture de la recherche ne invalide pas les requêtes
encore en vol, ce qui peut laisser une réponse ancienne réouvrir l’UI après
navigation. Dans closeSearch(), incrémentez searchSeqRef en plus de vider les
états, afin que les résultats déjà lancés deviennent obsolètes; gardez la
logique de recherche/filtrage dans les handlers associés à searchSeqRef pour
rejeter toute réponse avec un seq périmé.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b70cb93b-bf36-42fa-a67a-944a9339d688
📒 Files selected for processing (2)
docs/features/library.mdsrc/components/layout/TopBar.tsx
closeSearch() cleared the result state but didn't bump searchSeqRef, so a fan-out still in flight when the user clicked a result could resolve afterward and reopen the dropdown with stale results over the page just navigated to. Bump the seq in closeSearch so those responses are rejected by the existing `seq === searchSeqRef.current` guard.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/layout/TopBar.tsx`:
- Around line 246-250: La fermeture de la recherche invalide les requêtes en
cours, mais pas le debounce planifié par handleSearchInput, ce qui peut encore
déclencher runSearch après navigation. Dans TopBar.tsx, ajoute un moyen
d’annuler le setTimeout en attente lors de closeSearch (par exemple via une ref
de timer partagée avec handleSearchInput), et assure-toi que closeSearch nettoie
aussi ce timer en plus d’incrémenter searchSeqRef.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fecda811-51a4-473e-9bf7-8ad45553c699
📒 Files selected for processing (1)
src/components/layout/TopBar.tsx
closeSearch() invalidated in-flight requests but not the debounce timer scheduled by handleSearchInput, so a keystroke from just before a result click could still fire runSearch (with the captured old query) after navigation and reopen the dropdown. Clear searchTimerRef in closeSearch alongside the seq bump.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #321.
Problème
La barre de recherche annonce « titre, artiste, album » mais le dropdown ne listait que des titres — aucun moyen d'atteindre une page album ou artiste, alors que la donnée existe déjà.
Backend
search_albums/search_artists(browse.rs). FTS étant track-scoped, elles font un substring-match de la formecanonical_namede la requête contrealbum.canonical_title/artist.canonical_name(viainstr(), préfixes en tête), avecLIMITclampé. Même shape slim{ artwork_base, items }quelist_albums/list_artists→ le front réutiliseexpandAlbumRow/expandArtistRow.spawn_blocking) est factorisée en helpers partagésexpand_album_rows/expand_artist_rows, réutilisés par les chemins list et search (pas de duplication).Frontend
searchAlbums/searchArtists(browse.ts).Promise.all, dropdown en sections Artistes / Albums / Titres (composantSearchSection). Les rows artiste/album naviguent versArtistDetailView/AlbumDetailViewvia les nouvelles propsonNavigateToArtist/onNavigateToAlbum; les titres jouent comme avant.search_tracks_advancedtourne.i18n
topbar.search.{artists,albums,titles}ajoutés aux 17 locales.Docs
docs/features/library.md(section Search) + catalogueCLAUDE.md.Validation
bun run typecheck+bun run lint+cargo clippyverts. Pas de migration (FTS + colonnes canonical existent déjà).Summary by CodeRabbit