Fix deployed menu and search interactions#2
Conversation
Agent-Logs-Url: https://github.com/cmdicksonau/AsimoAudio/sessions/fcdb4c44-0f3f-420b-9c15-7834a97d0827 Co-authored-by: cmdicksonau <45134971+cmdicksonau@users.noreply.github.com>
Agent-Logs-Url: https://github.com/cmdicksonau/AsimoAudio/sessions/fcdb4c44-0f3f-420b-9c15-7834a97d0827 Co-authored-by: cmdicksonau <45134971+cmdicksonau@users.noreply.github.com>
Agent-Logs-Url: https://github.com/cmdicksonau/AsimoAudio/sessions/fcdb4c44-0f3f-420b-9c15-7834a97d0827 Co-authored-by: cmdicksonau <45134971+cmdicksonau@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses broken/deployed UI behavior in the React app by wiring up functional top-bar menu navigation and a working search experience that filters the library/collection views.
Changes:
- Added state + UI for a toggleable navigation menu (with backdrop close + Escape-to-close).
- Added a toggleable search bar and implemented query-based filtering of books/tracks.
- Added empty-state messaging for Library/Collection when no items match the search.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleEscape = (event: KeyboardEvent) => { | ||
| if (event.key !== 'Escape') return; | ||
| setIsMenuOpen(false); | ||
| setIsSearchOpen(false); |
There was a problem hiding this comment.
The Escape handler closes the search UI (setIsSearchOpen(false)) but leaves searchQuery intact, so the library/collection stay filtered while the query is hidden. Consider clearing searchQuery when closing search (Escape and/or toggle-off), or keeping the search bar visible/indicating an active filter when a query is present to avoid confusing “missing items” states.
| setIsSearchOpen(false); | |
| setIsSearchOpen(false); | |
| setSearchQuery(''); |
| <div className="mx-auto max-w-5xl"> | ||
| <input | ||
| value={searchQuery} | ||
| onChange={(event) => setSearchQuery(event.target.value)} |
There was a problem hiding this comment.
The search input relies on placeholder text only; it doesn’t have an accessible label. Add an explicit label (visible or screen-reader-only) or an aria-label/aria-labelledby so screen readers can announce what the field is for.
| onChange={(event) => setSearchQuery(event.target.value)} | |
| onChange={(event) => setSearchQuery(event.target.value)} | |
| aria-label="Search titles, authors, tags, and descriptions" |
| </button> | ||
| ) : ( | ||
| <button className="text-primary hover:text-white transition-colors"> | ||
| <button onClick={() => setIsMenuOpen((open) => !open)} className="text-primary hover:text-white transition-colors" aria-label="Open navigation menu"> |
There was a problem hiding this comment.
The menu icon button is a toggle, but the aria-label is always “Open navigation menu” and it doesn’t expose expanded state. Update the label to reflect open/closed state and add aria-expanded (and ideally aria-controls pointing at the menu) for better accessibility.
| <button onClick={() => setIsMenuOpen((open) => !open)} className="text-primary hover:text-white transition-colors" aria-label="Open navigation menu"> | |
| <button | |
| onClick={() => setIsMenuOpen((open) => !open)} | |
| className="text-primary hover:text-white transition-colors" | |
| aria-label={isMenuOpen ? 'Close navigation menu' : 'Open navigation menu'} | |
| aria-expanded={isMenuOpen} | |
| > |
| className="fixed inset-0 z-40 cursor-pointer bg-black/40" | ||
| aria-label="Close navigation menu" | ||
| /> | ||
| <div className="fixed left-4 top-20 z-50 w-60 rounded-xl border border-outline-variant/20 bg-slate-950/95 p-3 shadow-2xl backdrop-blur-xl"> |
There was a problem hiding this comment.
The backdrop for the open menu uses z-40 while the header/bottom nav use z-50, so users can still interact with those controls while the menu is open (e.g., navigate via bottom nav and leave the menu open). Consider increasing the backdrop z-index above the header/nav (and raising the menu panel above the backdrop), or otherwise ensuring navigation actions also close the menu.
| className="fixed inset-0 z-40 cursor-pointer bg-black/40" | |
| aria-label="Close navigation menu" | |
| /> | |
| <div className="fixed left-4 top-20 z-50 w-60 rounded-xl border border-outline-variant/20 bg-slate-950/95 p-3 shadow-2xl backdrop-blur-xl"> | |
| className="fixed inset-0 z-60 cursor-pointer bg-black/40" | |
| aria-label="Close navigation menu" | |
| /> | |
| <div className="fixed left-4 top-20 z-70 w-60 rounded-xl border border-outline-variant/20 bg-slate-950/95 p-3 shadow-2xl backdrop-blur-xl"> |
npm run lint,npm run build)