Repository navigation
fix(tokens): animate market cap period changes and reset the period on each open - #1584
Merged
Merged
Conversation
…n each open The currency-info overlay passes animateChartOpen = false to skip the chart's draw-in, but that nulled Vico's animationSpec, which also drives the animation between datasets. Period switches stopped animating. Keep animationSpec and put the flag on initialAnimationSpec, which only covers the first draw. The overlay reuses one TokenInfoViewModel across opens, so the selected period carried over from the last open. OnMintProvided now takes resetChartPeriod, which the overlay sets. The pushed TokenInfoScreen leaves it off because it re-dispatches OnMintProvided when a screen pushed over it pops, and resetting there would drop the user's pick. Chart data is keyed by period only, so a different mint now clears it. The period-selection flow no longer uses distinctUntilChanged: after a reset it would swallow re-picking the previous open's period. The tab row ignores taps on the selected period instead.
The chart's history load fires when the market cap arrives, behind two distinctUntilChanged calls keyed on the market cap alone. The overlay reuses one TokenInfoViewModel across opens, so opening a mint whose market cap equals the previous mint's skipped the load and the chart stayed empty until Retry or a period change. Key both de-dupes on the mint as well.
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.
Four market cap chart bugs in the currency-info overlay:
animateChartOpen = falseto skip the draw-in, and that set Vico'sanimationSpectonull.animationSpecalso drives the transition between datasets. The flag now controlsinitialAnimationSpec(first draw only), andanimationSpecstays attween(300).TokenInfoViewModelacross opens, soselectedPeriodpersisted.OnMintProvidedgainedresetChartPeriod, which the overlay sets on each open.distinctUntilChangedcalls keyed on the market cap alone, so the reused VM skipped it until Retry or a period change. Both now key on the mint too.runTransactionreturns. Switching ALL to 1W on the emulator showed the old all-time line in red for about 450 ms. The trend is now set afterrunTransactionreturns, and the line, area and marker colors fade over the same 300 ms as the curve. They're read at draw time, so the chart isn't rebuilt per frame; Vico'sAreaFill.singleand its indicator cache take fixed colors, so this addsVerticalGradientAreaFillandColorProviderComponent. A recording of the same switch shows the old line staying green until the morph starts, then fading through to red with it.Notes on the second fix:
TokenInfoScreenleavesresetChartPeriodoff. It re-dispatchesOnMintProvidedfromLaunchedEffect(Unit)when a screen pushed over it pops (e.g. Swap), and a reset there would throw away the period the user picked.historicalMarketCapDatais keyed by period only, so opening a different mint now clears it rather than briefly showing the previous mint's chart.distinctUntilChanged. Once the state resets to All, it would swallow re-picking the period from the previous open. The tab row ignores taps on the already-selected period instead, so there's no reload on a redundant tap.New reducer tests in
TokenInfoViewModelStateTestcover the reset, the default of keeping the period, and keeping versus clearing chart data by mint.TokenInfoViewModelChartLoadTestopens two mints with the same market cap on one VM and checks both load history; it failed on the second mint before the fix.