Repository navigation
Conversation
Searching for a legacy market rendered "No markets found": the match landed in the legacy group, which only renders when the "Show legacy markets" switch is on, and noResults never counted that group. An active search now reveals matching legacy markets regardless of the switch, and noResults accounts for them. Closes aave#3127
|
@Krshivam25 is attempting to deploy a commit to the Aave Team on Vercel. A member of the Team first needs to authorize it. |
5 tasks
This branch has not been deployed
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.
Searching for a legacy market rendered "No markets found": the match landed in the legacy group, which only renders when the "Show legacy markets" switch is on, and noResults never counted that group. An active search now reveals matching legacy markets regardless of the switch, and noResults accounts for them.
Closes #3127
General Changes
noResultsnow counts thelegacygroup, so a legacy-only match no longer renders the empty state.Developer Notes
Root cause — two things combined in
src/components/MarketSwitcher.tsx:legacygroup, but that section only renders when the "Show legacy markets" switch is on.noResultscountedpinned,ethereum,l2,other,v4Ethereumandv4Other— neverlegacy. With a legacy-only match every counted group is empty, so the empty state rendered on top of a match that existed.Both halves are needed: fixing only
noResultswould have replaced the wrong message with a blank panel, since the matching section still would not render.Approach — the two predicates are extracted to
src/components/marketSwitcher.utils.tsas pure functions with no imports, so they are unit-testable without rendering React or mocking the store.MarketSwitcher.tsxchanges in three places: the import, thenoResultsexpression, and the legacy section's render guard.Behaviour note — the switch keeps its meaning as a persistent preference, so it can read "off" while search results show legacy markets; clearing the search collapses them again. Happy to change this to only reveal legacy when it is the sole matching group if you'd prefer the narrower rule.
Tests —
src/components/__tests__/marketSwitcher.utils.test.ts, 8 pure-logic cases matching the repo's existing style, including the two regression guards (searched legacy market is shown; a legacy-only match is not reported as "no results").Scope — this does not touch
showLegacy's initialiser; that is #3128, which I'll send as a separate PR on top.Pre-existing failures — the 4 jest suites in
src/utils/that fail onmain(awagmi/chainsimport issue) are unaffected.Reviewer Checklist
Please ensure you, as the reviewer(s), have gone through this checklist to ensure that the code changes are ready to ship safely and to help mitigate any downstream issues that may occur.
.env.examplefile as well as the pertinant.github/actions/*files