Conversation
Remove hx-select-oob on view-filter links to prevent detaching the triggering element from the DOM. Use htmx:load event to re-apply search filtering after new content is loaded. Handle active filter class toggle in JS instead of server-rendered OOB swap.
There was a problem hiding this comment.
Pull request overview
This PR adds client-side search functionality to the entries list page, allowing users to filter entries by title, URL, description, and tags. The search input elegantly replaces the clock display in the navigation on hover/focus and can be activated with the / keyboard shortcut. The PR also fixes a bug where the search query was lost when switching between filters (Ready/Waiting/Unseen/All) by removing the htmx OOB swap mechanism and managing filter state client-side.
Changes:
- Added client-side search with real-time filtering using JavaScript
- Introduced search UI that swaps with the clock display on hover/focus
- Modified backend to fetch and include tags data for search functionality
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/entries/list.html | Added search UI, JavaScript for filtering, and filter state management |
| templates/entries/entry.html | Added data-search attribute to enable client-side filtering |
| templates/base.html | Wrapped clock in block to allow override |
| static/style.css | Added styles for search trigger and search field animations |
| src/routes/entries.rs | Added tag fetching logic and updated build_entry_view signature |
| src/routes/tags.rs | Updated to pass tags parameter to build_entry_view |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| input.addEventListener('input', function() { | ||
| clearTimeout(timer); | ||
| timer = setTimeout(filterEntries, 150); | ||
| }); | ||
|
|
||
| // re-apply search filter after htmx loads new content | ||
| document.body.addEventListener('htmx:load', function() { | ||
| if (input.value) filterEntries(); | ||
| }); |
There was a problem hiding this comment.
The htmx:load event listener is attached to document.body, which means it will fire for any htmx load event on the page, not just when the entry list is loaded. While this is functionally safe (it checks if input.value exists), it would be more efficient and clearer to attach the listener to the entry-list element specifically, or check if the loaded content is the entry list before calling filterEntries().
| input.addEventListener('input', function() { | |
| clearTimeout(timer); | |
| timer = setTimeout(filterEntries, 150); | |
| }); | |
| // re-apply search filter after htmx loads new content | |
| document.body.addEventListener('htmx:load', function() { | |
| if (input.value) filterEntries(); | |
| }); | |
| var entryList = document.getElementById('entry-list'); | |
| input.addEventListener('input', function() { | |
| clearTimeout(timer); | |
| timer = setTimeout(filterEntries, 150); | |
| }); | |
| // re-apply search filter after htmx loads new content for the entry list | |
| if (entryList) { | |
| entryList.addEventListener('htmx:load', function() { | |
| if (input.value) filterEntries(); | |
| }); | |
| } |
| <svg class="search-icon" width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><circle cx="11" cy="11" r="8"/><line x1="21" y1="21" x2="16.65" y2="16.65"/></svg> | ||
| <div class="search-swap"> | ||
| <span class="date" id="clock"></span> | ||
| <input type="search" class="search-field" id="search-input" placeholder="Search..." autocomplete="off" spellcheck="false"> |
There was a problem hiding this comment.
The search input field lacks an accessible label. While it has a placeholder, screen readers may not announce the purpose of this field clearly. Consider adding an aria-label attribute (e.g., aria-label="Search entries") to improve accessibility.
| <a href="/all" class="view-filter-link{% if filter == "all" %} active{% endif %}" hx-get="/all" hx-target="#entry-list" hx-select="#entry-list" hx-select-oob="#view-filter" hx-swap="outerHTML" hx-push-url="true">All</a> | ||
| {% block nav_end %} | ||
| <div class="search-trigger" id="search-trigger"> | ||
| <svg class="search-icon" width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><circle cx="11" cy="11" r="8"/><line x1="21" y1="21" x2="16.65" y2="16.65"/></svg> |
There was a problem hiding this comment.
The search icon SVG lacks an accessible label. Screen reader users won't know what this icon represents. Consider adding role="img" and aria-label="Search" to the SVG element, or wrap it in a button with an accessible label.
| var q = input.value.toLowerCase(); | ||
| var entries = document.getElementById('entry-list').querySelectorAll('.entry'); | ||
| for (var i = 0; i < entries.length; i++) { | ||
| var haystack = entries[i].getAttribute('data-search').toLowerCase(); |
There was a problem hiding this comment.
The filterEntries function will throw an error if an entry element doesn't have a data-search attribute (getAttribute returns null, and calling toLowerCase() on null will throw). Consider adding a null check or using getAttribute('data-search') || '' to provide a default value.
| var haystack = entries[i].getAttribute('data-search').toLowerCase(); | |
| var haystack = (entries[i].getAttribute('data-search') || '').toLowerCase(); |
| @@ -164,7 +164,7 @@ async fn show_tag( | |||
| .into_iter() | |||
| .map(|r| { | |||
| let (entry, count) = r.into_entry_and_count(); | |||
There was a problem hiding this comment.
The tags parameter passed to build_entry_view only includes the current tag being viewed, not all tags associated with the entry. This means the data-search attribute in the rendered entry.html will only include one tag, making the search functionality incomplete when viewing entries from a tag page. Consider fetching all tags for these entries using fetch_tags_for_entries similar to how it's done in list_filtered_entries, or document this limitation.
| let (entry, count) = r.into_entry_and_count(); | |
| let (entry, count) = r.into_entry_and_count(); | |
| // NOTE: On tag pages we only pass the current tag (`name`) here. | |
| // This means the data-search attribute for these entries will only | |
| // include this tag, not all tags associated with the entry. |
Summary
/shortcutDetails
The filter links previously used
hx-select-oobto OOB-swap#view-filter, which detached the triggering<a>from the DOM before htmx events could bubble. This brokehtmx:afterSettleand prevented search re-application after filter switches.Fix: removed OOB swap, use
htmx:loadto re-apply search after new content loads, and handle the active filter class in JS.Known issues
htmx:loadfires and re-applies the filter.htmx:afterSwap/htmx:afterSettlefire earlier but don't work reliably withouterHTMLswaps.Test plan
/keyboard shortcut focuses search input