fix(frontend): show the search dropdown's loading state while a search runs - #1072
Merged
Merged
Conversation
Frontend CI, Lighthouse CI and Playwright E2E had all been failing on main
for several merges. Each job died at a different point; this fixes the
whole chain so the suite runs green.
Build / lint (blocked Lighthouse CI and E2E, which both run `npm run build`)
- escape unescaped quotes and apostrophes in privacy/ and terms/ JSX
(react/no-unescaped-entities, 22 errors) using entities that render
identically, so the pages are unchanged visually
- dashboard: the dominant-asset reduce read `.value`/`.name`, which do not
exist on AssetBreakdownEntry; use `.amount`/`.assetCode` as the chart's
own dataKey/nameKey already do. This was a type error that only surfaced
once lint stopped failing first.
Runtime CSP (all client-side JS was blocked on every static route)
- `script-src 'self' 'nonce-…'` cannot work with statically prerendered
pages: their HTML is built ahead of time and cannot carry a per-request
nonce, and a nonce makes browsers ignore 'unsafe-inline'. Every static
page therefore loaded with no working JavaScript. Drop the nonce and
allow inline scripts. JSON-LD is `application/ld+json`, which is not
executable and was never gated by script-src, so its nonce goes too.
- build connect-src from lib/config instead of raw process.env: the client
falls back to its own defaults (localhost:4001, Horizon, Soroban) that
the policy then omitted, so every API request was refused.
Test suite (50 failures -> 0; 106 tests pass)
- vitest was collecting the Playwright specs in e2e/; exclude them
- add the missing @testing-library/user-event dev dependency
- replace full-module vi.mock factories with importOriginal-based partial
mocks, so a stub no longer erases exports that transitive imports need
- mock next/navigation and focus-trap-react once in the shared setup
- make the framer-motion stubs cover any motion.<tag> via a Proxy
- wrap ActivityFeed snapshots in a QueryClientProvider
- pin Math.random for the MilestoneCard confetti so its snapshot is stable
- update assertions that described removed behaviour: the support panel
reports wallet errors through a toast and opens the result modal, copy
shows inline feedback, ProfileTabs empty-state copy is no longer
personalised, and the create wizard collects the wallet on step 2
- restore a funded-account stub between support-panel tests, since
clearAllMocks() clears calls but not implementations
- fix queries that matched several elements ("Failed", "XLM" vs "yXLM")
- commit the previously untracked snapshots; vitest does not write new
ones under CI and fails instead
Also expose aria-pressed on the create wizard's asset toggles, which the
existing test asserted and screen readers need, and stub the profile API
in the home E2E spec so it does not depend on a live backend.
…h runs showDropdown was only set inside the fetch's success branch, so for a fresh search — the common case, with the dropdown closed — isSearching stayed true for the whole debounce-plus-fetch while showDropdown stayed false and nothing rendered. The dropdown then opened in the same tick that cleared isSearching, so "Searching..." was never painted and results simply appeared with no loading feedback. Open the dropdown as soon as the query is non-empty, before the debounce starts, so the in-flight state is actually visible. The success branch's setShowDropdown(true) is now redundant and drops out, which also means a dropdown the user dismissed mid-request stays dismissed instead of springing back open when results land. Failed lookups still close the dropdown rather than falling through to "No results found", which would misreport a network error as an empty result. Adds tests covering the loading state during the debounce and the request, the handoff to results, the empty-result and cleared-query cases, and the failure path. Closes OlaGreat#1063
|
@DSOTec Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
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.
Problem
In
app-shell.tsx,showDropdownwas only ever set totrueinside the search fetch's success branch — never before or during the request. The dropdown itself is gated onshowDropdown, and the "Searching..." text lives inside it, gated onisSearching:So for any fresh search — the common case, where the dropdown starts closed —
isSearchingwastruefor the whole debounce-plus-fetch duration whileshowDropdownstayedfalse, and nothing rendered at all. By the timesetShowDropdown(true)ran,setIsSearching(false)ran in the same tick and was batched with it, so the loading branch was never painted. Users never saw "Searching..."; results just popped in with no feedback.Solution
Open the dropdown as soon as the query is non-empty, before the debounce starts, so the in-flight state is actually visible:
Two follow-on details:
setShowDropdown(true)is now redundant and drops out. A side benefit: a dropdown the user dismissed mid-request (Escape or click-outside) now stays dismissed instead of springing back open when results land.setShowDropdown(false). I deliberately kept this: letting a failure fall through to the open dropdown would render "No results found", misreporting a network error as an empty result set.Behaviour on the success path, empty results, and a cleared query is unchanged.
Testing
Added
frontend/src/components/app-shell.test.tsxwith 6 tests:fetchdouble that never settles on its own, so the pending state can be observed directlyVerified the tests actually catch the bug: against the code before this change, tests 1, 2 and 6 fail (
Unable to find an element with the text: Searching...) while 3, 4 and 5 still pass — confirming the fix targets the loading state without regressing the paths that already worked.Full suite from a clean
npm ci, matching what CI runs:Note on the first commit
This branch carries two commits. The second (
fix(frontend): show the search dropdown's loading state...) is the actual fix for #1063 and is a 6-line change.The first commit is the CI-pipeline repair already submitted as #1071, included here only because
mainis currently red —Frontend CI,Lighthouse CIandE2E (Playwright)all fail onmain, so a branch based on it cannot go green on its own. If #1071 merges first, I'll rebase and that commit disappears from this PR, leaving just theapp-shell.tsxchange and its tests. Reviewingfrontend/src/components/app-shell.tsxandapp-shell.test.tsxalone gives the complete picture of this fix.Closes #1063