feat(Templates): add search functionality for user templates #901
Conversation
✅ Deploy Preview for livecodes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdded a templates search UI to the templates tab header with a labeled input field. Implemented debounced keystroke handling (150ms) that emits a "templates:filter" CustomEvent on the templates container element for template filtering. Changes
Sequence DiagramsequenceDiagram
participant User
participant Input as Search Input
participant Debounce as Debounce Logic
participant EventTarget as #templates-container
participant Listener as Filter Handler
User->>Input: Types search query
Input->>Debounce: Keystroke (rapid fire)
Note over Debounce: 150ms debounce window
Debounce->>EventTarget: Emit CustomEvent("templates:filter")<br/>with query in event.detail
EventTarget->>Listener: Dispatch event
Listener->>Listener: Filter templates based on query
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/livecodes/html/templates.html(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Redirect rules - livecodes
- GitHub Check: Header rules - livecodes
- GitHub Check: Pages changed - livecodes
🔇 Additional comments (1)
src/livecodes/html/templates.html (1)
36-55: Script logic is sound; confirm filtering implementation is planned.The inline script correctly debounces input (150ms), emits a
templates:filtercustom event with the query value, and uses defensive null checks. The implementation is event-driven as intended, allowing other scripts to listen and filter templates accordingly.Since this is described as a demo PR pending actual filtering logic, verify that:
- Event listeners consuming the
templates:filterevent will be implemented separately.- The event dispatch to
#templates-containeraligns with where filter listeners will be attached.
| <li class="templates-search"> | ||
| <label for="templates-search-input" class="visually-hidden" data-i18n="templates.search.label">Search templates</label> | ||
| <input id="templates-search-input" type="search" placeholder="Filter languages..." aria-label="Filter templates by language" /> | ||
| </li> |
There was a problem hiding this comment.
Move search input out of the tab list for proper semantic structure.
Placing the search input as an <li> inside the tab navigation list violates WAI-ARIA tab patterns. The tab list (<ul class="modal-tabs">) contains tab controls, not search fields. Screen readers will incorrectly announce this as a tab item rather than a search field, degrading accessibility.
Recommend restructuring to place the search input in a dedicated container outside the tab list (e.g., above or below the tabs, or in a separate section).
Consider restructuring like this:
<ul id="templates-tabs" class="modal-tabs">
<li class="active">
<a href="#" data-target="templates-starter" data-i18n="templates.starter.heading">
Starter Templates
</a>
</li>
<li>
<a href="#" data-target="templates-user" data-i18n="templates.user.heading">My Templates</a>
</li>
- <li class="templates-search">
- <label for="templates-search-input" class="visually-hidden" data-i18n="templates.search.label">Search templates</label>
- <input id="templates-search-input" type="search" placeholder="Filter languages..." aria-label="Filter templates by language" />
- </li>
</ul>
+ <div class="templates-search-container">
+ <label for="templates-search-input" class="visually-hidden" data-i18n="templates.search.label">Search templates</label>
+ <input id="templates-search-input" type="search" placeholder="Filter languages..." aria-label="Filter templates by language" />
+ </div>Adjust CSS styling as needed to position the search container appropriately within the templates modal layout.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/livecodes/html/templates.html around lines 13 to 16, the search input is
currently an <li> inside the tab list which breaks WAI-ARIA tab semantics; move
the search input out of the <ul class="modal-tabs"> into its own container (for
example a <div class="templates-search">) placed immediately before or after the
tab list, keep the existing label, input id, placeholder and aria-label intact,
and remove the <li> wrapper; finally update/adjust CSS selectors to position the
new .templates-search container as needed within the templates modal layout so
visual placement remains unchanged.
There was a problem hiding this comment.
I think the search input should not be added as a new tab, since it affects the other tabs. The suggestion by CodeRabbit AI is reasonable.
There was a problem hiding this comment.
@hatemhosny Thank you for confirming! Moving the search input outside the tab list will maintain proper semantic structure and ensure the tabs remain independent of the filtering functionality. The suggested refactoring will improve both accessibility and the overall architecture of the component.
| <li class="templates-search"> | ||
| <label for="templates-search-input" class="visually-hidden" data-i18n="templates.search.label">Search templates</label> | ||
| <input id="templates-search-input" type="search" placeholder="Filter languages..." aria-label="Filter templates by language" /> | ||
| </li> |
There was a problem hiding this comment.
I think the search input should not be added as a new tab, since it affects the other tabs. The suggestion by CodeRabbit AI is reasonable.
| </div> | ||
|
|
||
| <!-- Inline: emit a "templates:filter" event with the current query so other scripts can perform filtering --> | ||
| <script> |
There was a problem hiding this comment.
You can move the logic to src/livecodes/UI/templates.ts.
Think of the html here as the "view" while the logic in templates.ts as the "controller" in MVC pattern.
Also if you do that, you can directly manipulate DOM elements e.g. add display: none; without having to emit events.
| var val = e.target.value || ''; | ||
| // debounce to avoid excessive events while typing | ||
| clearTimeout(timeout); | ||
| timeout = setTimeout(function () { emit(val.trim()); }, 150); |
There was a problem hiding this comment.
There is a debounce function here that you can re-use.
| </li> | ||
| <li class="templates-search"> | ||
| <label for="templates-search-input" class="visually-hidden" data-i18n="templates.search.label">Search templates</label> | ||
| <input id="templates-search-input" type="search" placeholder="Filter languages..." aria-label="Filter templates by language" /> |
There was a problem hiding this comment.
Let's change "Filter languages..." to "Search templates...". And then we can search by title, languages and others (e.g. tags).
Also, after you finish your edits, please run npm run i18n-export to regenerate the i18n json and fix this build error.
|
Thank you @DevAbdelrahmanSayed |
|
Hi, thanks for mentioning me in the comments! I’d love to work on and finish this, but I might not be able to get to it before the weekend. Just a general note, it’s usually nice to check in with the PR contributor first before escalating to the maintainers. It tends to come across as more polite and professional. If the contributor doesn’t respond for a while and the PR becomes stale, then escalating to the maintainers would make sense. |
2 participants
(This is a demo PR, will check the functionality later today)
This pull request enhances the templates UI by adding a search input for filtering templates by language and introduces an event-driven mechanism for template filtering. The main changes are grouped below:
UI Enhancements:
<input id="templates-search-input" ...>) to the templates navigation, allowing users to filter templates by language.Event-Driven Filtering:
templates:filtercustom event with the current query. This enables other scripts to react and perform the actual filtering logic.What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
Fixes #900
This pull request adds a search/filter feature to the templates section in the
templates.htmlfile, allowing users to filter templates by language. It introduces a search input UI and emits a custom event to enable template filtering functionality.User interface improvements:
<input id="templates-search-input" ...>) to the templates navigation for filtering templates by language.Interactivity and event handling:
templates:filtercustom event with the current query to enable live filtering of templates.Mobile & Desktop Screenshots/Recordings
Added tests?
Added to documentations?
[optional] Are there any post-deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit