MXWAR-75 feat: implement dashboard client search dropdown#87
Open
VivekSingla20 wants to merge 1 commit intoopenMF:devfrom
Open
MXWAR-75 feat: implement dashboard client search dropdown#87VivekSingla20 wants to merge 1 commit intoopenMF:devfrom
VivekSingla20 wants to merge 1 commit intoopenMF:devfrom
Conversation
📝 WalkthroughWalkthroughExtracts the Dashboard's inline search into a new DashboardSearch component. Dashboard now imports and renders DashboardSearch; CardContent layout updated. Adds a localized search string bundle. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dashboard as Dashboard
participant Search as DashboardSearch
participant API as ClientSearchV2Api
participant Router as React Router
User->>Search: Type query
Search->>Search: Debounce (250ms)
Search->>API: searchByText(query)
API-->>Search: results / error
Search->>User: Show dropdown (loading, results, or error)
User->>Search: Arrow keys / hover / Enter / Click
Search->>Router: navigate to /clients/{id}/general
Router-->>User: Show client details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 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)
📝 Coding Plan
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 Tip You can disable the changed files summary in the walkthrough.Disable the |
![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 the current code and only fix it if needed.
Inline comments:
In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx`:
- Around line 31-33: The loading spinner never appears on the first query
because you set isLoading before the popup mounts (the popup only renders when
isOpen is true); update the timer callback inside DashboardSearch so you call
setIsOpen(true) before setting setIsLoading(true) (and still clear/set error as
needed) so the popup is mounted when loading starts; apply the same ordering
change to the similar block around the 127-136 region so both places open the
popup first, then set loading/error state.
- Around line 104-110: The Label and Input in DashboardSearch currently use
hard-coded English strings (e.g., the "Search Clients" label, "Search by name,
ID or mobile..." placeholder) and other strings in the component (loading/error
text, empty-state copy and fallback labels between the DashboardSearch render
and the subsequent block at lines ~135–182); replace all hard-coded UI copy with
the project’s existing translation flow: import and use the same translation
helper used elsewhere in the dashboard (e.g., the useTranslation/t or translate
function), swap the Label text, Input placeholder, loading/error messages,
empty-state text and any fallback labels to translation keys (e.g.,
dashboard.search.label, dashboard.search.placeholder, dashboard.search.loading,
dashboard.search.error, dashboard.search.empty, dashboard.search.fallback) and
ensure you provide sensible keys and fallbacks via the translation call so
missing keys degrade gracefully.
- Around line 71-89: The Escape key is currently blocked by the early return
that checks results.length === 0 in handleKeyDown; change the handler so Escape
dismissal runs regardless of results length: first check if isOpen and e.key ===
'Escape' and call setIsOpen(false), then return; keep the existing guard (if
!isOpen or results.length === 0) or move it to only gate navigation keys
(ArrowDown, ArrowUp, Enter) so arrow/enter logic still requires results, while
Escape always closes the popup.
- Around line 22-54: Older async search responses can overwrite newer state
because only the debounce timer is cleared; modify the effect to ignore
out-of-date responses by tracking a request token: create a ref (e.g.,
latestRequestRef) or local requestId that you increment before calling
searchApi.searchByText inside the useEffect, capture that token in the async
closure, and before calling setResults/setIsOpen/setError/setIsLoading check
that the token matches latestRequestRef.current (or that the request wasn't
aborted); alternatively, if searchApi.searchByText accepts an AbortSignal,
create an AbortController per request, pass its signal, and call
controller.abort() in the effect cleanup to cancel in-flight requests — update
the useEffect (and any finally/error branches) to bail out when the response is
stale/aborted so old responses do not reopen the dropdown or overwrite results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c4af208-f6b2-4e8e-874c-6c594a10dfd2
📒 Files selected for processing (2)
src/pages/home/dashboard/Dashboard.tsxsrc/pages/home/dashboard/dashboard-search/DashboardSearch.tsx
Comment on lines
+31
to
+33
| const timer = setTimeout(async () => { | ||
| setIsLoading(true) | ||
| setError(null) |
There was a problem hiding this comment.
Render the loading panel for the first search.
isLoading is set before the response arrives, but the popup only mounts when isOpen is true. On the initial query the spinner never appears, so the field looks idle until the request finishes.
Small render-condition fix
- {isOpen && query.trim() && (
+ {(isOpen || isLoading) && query.trim() && (Also applies to: 127-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx` around lines
31 - 33, The loading spinner never appears on the first query because you set
isLoading before the popup mounts (the popup only renders when isOpen is true);
update the timer callback inside DashboardSearch so you call setIsOpen(true)
before setting setIsLoading(true) (and still clear/set error as needed) so the
popup is mounted when loading starts; apply the same ordering change to the
similar block around the 127-136 region so both places open the popup first,
then set loading/error state.
VivekSingla20
force-pushed
the
MXWAR-75-dashboard-client-search
branch
from
d33b6b0 to
1912f0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx (1)
25-43:⚠️ Potential issue | 🟡 MinorUse normalized query text for the API request too.
The guard trims input, but the request sends untrimmed
query. This can produce avoidable no-result responses for whitespace-padded input.Suggested fix
useEffect(() => { - if (!query.trim()) { + const text = query.trim() + if (!text) { setResults([]) setIsOpen(false) setSelectedIndex(-1) setError(null) setIsLoading(false) return } ... const res = await searchApi.searchByText({ - request: { text: query }, + request: { text }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx` around lines 25 - 43, The code trims input in the guard but still sends the original untrimmed query to searchApi.searchByText, causing whitespace-padded queries to misbehave; update DashboardSearch to compute a normalized variable (e.g., const trimmedQuery = query.trim()) and use that for the empty check and when calling searchApi.searchByText (request: { text: trimmedQuery }); ensure setResults/setIsOpen/setSelectedIndex/setError/setIsLoading still use the same logic but the API request and any other downstream usage reference trimmedQuery instead of query.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx`:
- Around line 128-130: The combobox currently always sets aria-controls even
when the results listbox isn’t rendered; update the JSX in DashboardSearch (the
element with role="combobox" and the other input/combobox attributes) to only
include aria-controls when the listbox exists (e.g. use a conditional expression
like isOpen ? "client-search-results" : undefined or omit the prop when closed)
and apply the same conditional change for the other occurrence (the mirrored
attributes around the list rendering). Ensure the attribute name and value
remain exactly "aria-controls" and "client-search-results" when the listbox is
rendered so the accessibility relationship is preserved.
- Line 11: The module-scoped instantiation const searchApi = new
ClientSearchV2Api(getConfiguration()) snapshots headers at import-time and can
become stale; instead, construct the ClientSearchV2Api inside the request path
(e.g., inside the DashboardSearch component's search handler or the
performSearch/useSearch function) or provide a factory that calls
getConfiguration() per request so each call uses current headers; replace
references to the module-scoped searchApi with a locally constructed client (new
ClientSearchV2Api(getConfiguration())) at call time to ensure fresh auth
headers.
---
Duplicate comments:
In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx`:
- Around line 25-43: The code trims input in the guard but still sends the
original untrimmed query to searchApi.searchByText, causing whitespace-padded
queries to misbehave; update DashboardSearch to compute a normalized variable
(e.g., const trimmedQuery = query.trim()) and use that for the empty check and
when calling searchApi.searchByText (request: { text: trimmedQuery }); ensure
setResults/setIsOpen/setSelectedIndex/setError/setIsLoading still use the same
logic but the API request and any other downstream usage reference trimmedQuery
instead of query.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e8dc8f5-0098-470e-980f-01a0f3fb0b65
📒 Files selected for processing (3)
src/locales/en-US/common.jsonsrc/pages/home/dashboard/Dashboard.tsxsrc/pages/home/dashboard/dashboard-search/DashboardSearch.tsx
| import type { ClientSearchData } from '@/fineract-api' | ||
| import { getConfiguration } from '@/lib/fineract-openapi' | ||
|
|
||
| const searchApi = new ClientSearchV2Api(getConfiguration()) |
There was a problem hiding this comment.
Avoid module-scoped API client with snapshotted headers.
getConfiguration() is executed once at import time here, so request headers can become stale during long-lived sessions. Create the API client when executing the request (or use a refresh-safe factory) so each call uses current headers.
Suggested fix
-const searchApi = new ClientSearchV2Api(getConfiguration())
...
- const res = await searchApi.searchByText({
+ const res = await new ClientSearchV2Api(getConfiguration()).searchByText({
request: { text: query },
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx` at line 11,
The module-scoped instantiation const searchApi = new
ClientSearchV2Api(getConfiguration()) snapshots headers at import-time and can
become stale; instead, construct the ClientSearchV2Api inside the request path
(e.g., inside the DashboardSearch component's search handler or the
performSearch/useSearch function) or provide a factory that calls
getConfiguration() per request so each call uses current headers; replace
references to the module-scoped searchApi with a locally constructed client (new
ClientSearchV2Api(getConfiguration())) at call time to ensure fresh auth
headers.
Comment on lines
+128
to
+130
| role="combobox" | ||
| aria-expanded={isOpen} | ||
| aria-controls="client-search-results" |
There was a problem hiding this comment.
Make aria-controls conditional to rendered listbox state.
aria-controls is always set, but the controlled listbox only exists in the results branch. Set it only when the listbox is actually rendered.
Suggested fix
<Input
id="dashboard-search"
...
- aria-controls="client-search-results"
+ aria-controls={isOpen && results.length > 0 ? 'client-search-results' : undefined}Also applies to: 154-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/home/dashboard/dashboard-search/DashboardSearch.tsx` around lines
128 - 130, The combobox currently always sets aria-controls even when the
results listbox isn’t rendered; update the JSX in DashboardSearch (the element
with role="combobox" and the other input/combobox attributes) to only include
aria-controls when the listbox exists (e.g. use a conditional expression like
isOpen ? "client-search-results" : undefined or omit the prop when closed) and
apply the same conditional change for the other occurrence (the mirrored
attributes around the list rendering). Ensure the attribute name and value
remain exactly "aria-controls" and "client-search-results" when the listbox is
rendered so the accessibility relationship is preserved.
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.
Description
Improved the client search experience by adding debounce, keyboard navigation, proper TypeScript typing, ARIA accessibility attributes, and error state handling for a more responsive and accessible search dropdown.
This enhancement wires up the ClientSearchV2Api to this input field to provide a functional, accessible client search experience directly from the dashboard.
Related issues and discussion
#MXWAR-75
Before Changes:
before.mp4
After Changes:
after.mp4
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
CONTRIBUTING.md.Summary by CodeRabbit