feat: add search autocomplete dropdown for courses#1007
Open
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Open
feat: add search autocomplete dropdown for courses#1007ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a client-side search autocomplete: template changes and JS to fetch suggestions, a new URL route, and a backend view that returns up to 8 published course suggestions as JSON. The backend view is duplicated in the file (two identical definitions). Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser as Browser (JS)
participant Server as Django Server
participant DB as Database
User->>Browser: Type query in search input
Browser->>Browser: Debounce input
Browser->>Server: GET /search/autocomplete/?q={query}
Server->>DB: Query published courses (title/tags/teacher match)
DB-->>Server: Return up to 8 course rows
Server-->>Browser: JSON {"results":[...]}
Browser->>Browser: Render dropdown items
Browser->>User: Show autocomplete dropdown
User->>Browser: Press Escape or click outside
Browser->>Browser: Hide dropdown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/base.html`:
- Around line 1149-1164: The dropdown generated in the data.results.forEach loop
lacks ARIA roles and attributes; update the code that creates the dropdown (see
dropdown.replaceChildren() and the loop creating the variable a) to set
role="listbox" on the dropdown container and role="option" on each suggestion
element (the anchor variable a), add appropriate aria-selected on the active
item, and ensure the associated input element has aria-expanded and
aria-controls pointing to the dropdown ID so screen readers can detect and
control the suggestions.
- Around line 1141-1166: The fetch-based autocomplete call lacks error handling:
wrap the promise chain starting at the fetch("{% url 'search_autocomplete' %}" +
'?q=' + encodeURIComponent(query)) in a .catch handler (or add a final .catch)
that logs the error (e.g., console.error or a UI logger), clears or hides the
dropdown (use dropdown.classList.add('hidden') and dropdown.innerHTML = ''), and
ensures no stale UI remains; update the block that manipulates dropdown
(creation in the .then callbacks and dropdown.classList.remove('hidden')) so the
.catch performs the cleanup on failure.
- Around line 359-360: Move the <div id="autocomplete-dropdown"> so it is nested
inside the search container element that has the relative positioning (the
element surrounding the search input with class "relative"), ensuring the
absolute-positioned dropdown is a child of that relative parent; remove the
stray extra closing </div> that currently closes the parent too early to prevent
breaking the navbar layout and verify the dropdown div sits before the search
container's closing tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f015dfd-cd0f-46c8-b774-15df4bbde767
📒 Files selected for processing (3)
web/templates/base.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/search-autocomplete
branch
from
8946e9e to
61d903a
Compare
Author
|
Addressed all three CodeRabbit comments:
|
ayesha1145
force-pushed
the
feat/search-autocomplete
branch
from
61d903a to
8e89f13
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/templates/base.html (1)
348-349:⚠️ Potential issue | 🟡 MinorAdd combobox ARIA linkage/state on the input.
Line 358 sets
role="listbox"and Line 1152 sets option roles, but the input (Lines 348-349) still doesn’t expose combobox state (aria-controls/aria-expanded), so screen readers won’t get complete autocomplete context.♿ Suggested patch
- <input type="text" - id="search-input-desktop" - autocomplete="off" + <input type="text" + id="search-input-desktop" + autocomplete="off" + role="combobox" + aria-autocomplete="list" + aria-controls="autocomplete-dropdown" + aria-expanded="false" name="q" placeholder="What do you want to learn?" class="rounded-full w-[250px] bg-white dark:bg-gray-700 text-gray-900 dark:text-gray-100 px-3 py-1.5 focus:outline-none focus:ring-2 focus:ring-teal-300 dark:focus:ring-teal-700" /> @@ if (query.length < 2) { dropdown.classList.add('hidden'); dropdown.innerHTML = ''; + input.setAttribute('aria-expanded', 'false'); return; } @@ dropdown.classList.remove('hidden'); + input.setAttribute('aria-expanded', 'true'); }) .catch(() => { dropdown.classList.add('hidden'); dropdown.replaceChildren(); + input.setAttribute('aria-expanded', 'false'); }); @@ if (!input.contains(e.target) && !dropdown.contains(e.target)) { dropdown.classList.add('hidden'); + input.setAttribute('aria-expanded', 'false'); } @@ - if (e.key === 'Escape') dropdown.classList.add('hidden'); + if (e.key === 'Escape') { + dropdown.classList.add('hidden'); + input.setAttribute('aria-expanded', 'false'); + }As per coding guidelines: "Include proper ARIA labels where needed for accessibility".
Also applies to: 358-358, 1135-1137, 1165-1182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/base.html` around lines 348 - 349, The input element with id "search-input-desktop" is missing combobox ARIA attributes—add aria-controls pointing to the listbox element id and add a dynamic aria-expanded (true/false) on "search-input-desktop"; also ensure aria-activedescendant is set/cleared to match the currently focused option when navigating options (the listbox uses role="listbox" and the options use role="option"), and update these attributes whenever the suggestion dropdown opens, closes, or the active option changes so screen readers receive proper combobox state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/base.html`:
- Around line 1139-1170: The async fetch for autocomplete can apply out-of-order
responses; fix by introducing request cancellation or staleness checks: add an
outer-scope AbortController (e.g., autocompleteController) or a monotonic
request id (e.g., latestRequestId) and before starting the fetch abort the
previous controller or increment the id and capture it for this request; then
pass the controller.signal into fetch (fetch(url, { signal })) and on success
only update dropdown if the response matches the current controller (not
aborted) or the captured request id equals latestRequestId; ensure you clean up
the controller on catch/abort and keep using existing debounceTimer logic.
---
Duplicate comments:
In `@web/templates/base.html`:
- Around line 348-349: The input element with id "search-input-desktop" is
missing combobox ARIA attributes—add aria-controls pointing to the listbox
element id and add a dynamic aria-expanded (true/false) on
"search-input-desktop"; also ensure aria-activedescendant is set/cleared to
match the currently focused option when navigating options (the listbox uses
role="listbox" and the options use role="option"), and update these attributes
whenever the suggestion dropdown opens, closes, or the active option changes so
screen readers receive proper combobox state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f6fc7b0-097e-4973-85f3-de4632e11619
📒 Files selected for processing (3)
web/templates/base.htmlweb/urls.pyweb/views.py
github-actions
bot
dismissed
their stale review
All review conversations have been resolved.
ayesha1145
force-pushed
the
feat/search-autocomplete
branch
from
8e89f13 to
b3cc661
Compare
Contributor
👀 Peer Review RequiredHi @ayesha1145! This pull request does not yet have a peer review. Before this PR can be merged, please request a review from one of your peers:
Thank you for contributing! 🎉 |
Author
|
Added AbortController to cancel stale in-flight requests when a new keystroke fires before the previous fetch completes. |
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.
Adds a live search autocomplete dropdown to the desktop navbar search input.
web/views.py: newsearch_autocompleteview with@require_GET, type hints, docstring, server-side 64-char query guard,.order_by("title"), andreverse()for URLsweb/urls.py: new routesearch/autocomplete/web/templates/base.html: dropdown div inside search container, debounced fetch using{% url %}tag, safe DOM construction (no innerHTML), Escape key to closeSummary by CodeRabbit
New Features
Bug Fixes / UI