Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function PriceImpactIndicator({ priceImpactParams, isBridging = false }:

return (
<span>
{priceImpact ? (
{priceImpact && !priceImpactLoading ? (
<PriceImpactWrapper priceImpact$={priceImpact} isBridging$={isBridging}>
{' '}
<HoverTooltip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Fraction, Percent } from '@cowprotocol/currency'
import ms from 'ms.macro'

import { useDerivedTradeState } from 'modules/trade'
import { useTradeQuote } from 'modules/tradeQuote'
import { useTradeUsdAmounts } from 'modules/usdAmount'

import { useSafeMemo } from 'common/hooks/useSafeMemo'
Expand All @@ -31,7 +32,12 @@ export function useFiatValuePriceImpact(): { priceImpact: Percent | undefined; i
outputAmount: { value: fiatValueOutput, isLoading: outputIsLoading },
} = useTradeUsdAmounts(inputCurrencyAmount, outputCurrencyAmount, inputToken, outputToken)

const isLoading = inputIsLoading || outputIsLoading
const { isLoading: isQuoteLoading, hasParamsChanged: quoteParamsChanged } = useTradeQuote()

// Trade-quote signals indicate the current output amount is stale (token just changed
// or a fresh quote is in flight). Compute price impact only once the quote catches up,
// otherwise we'd display a huge nonsense % derived from mismatched in/out amounts.
const isLoading = inputIsLoading || outputIsLoading || isQuoteLoading || quoteParamsChanged
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const [hasLoadingTimedOut, setHasLoadingTimedOut] = useState(false)

useEffect(() => {
Expand All @@ -51,12 +57,20 @@ export function useFiatValuePriceImpact(): { priceImpact: Percent | undefined; i
// Don't calculate price impact if trade is not set up (both trade assets are not set)
if (!isTradeSetUp) return null

const stillLoading = isLoading && !hasLoadingTimedOut

// While a fresh quote is loading, don't expose the stale value at all — consumers
// hide the percentage when `priceImpact` is undefined, leaving just the spinner.
if (stillLoading) {
return { priceImpact: undefined, isLoading: true }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While a fresh quote is loading, don't expose the stale value at all —

This never happens because after the first quote is being loaded, the second quote (the fresh quote as you say) has hasLoadingTimedOut === true so stillLoading is never true again, after the first quote.

So this code and comment are incorrect

}

const priceImpact = computeFiatValuePriceImpact(
fiatValueInput ? FractionUtils.fractionLikeToFraction(fiatValueInput) : null,
fiatValueOutput ? FractionUtils.fractionLikeToFraction(fiatValueOutput) : null,
)

return { priceImpact, isLoading: isLoading && !hasLoadingTimedOut }
return { priceImpact, isLoading: false }
}, [isTradeSetUp, fiatValueInput, fiatValueOutput, isLoading, hasLoadingTimedOut])
}

Expand Down
Loading