feat: reset inputs after submission #165
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| // TODO(zzmp): Surface errors from wrap. | ||
| console.log(e) | ||
| } finally { | ||
| setIsPending(false) |
There was a problem hiding this comment.
onSubmit will always return, so a finally is no longer needed.
| invariant(trade.trade) | ||
| return { type: TransactionType.SWAP, response, tradeType: trade.trade.tradeType, trade: trade.trade } | ||
| }) | ||
| setOpen(false) |
There was a problem hiding this comment.
This should only be called in the case of a successful submission.
| setDisplayTxHash(transaction.hash) | ||
| const submitted = await onSubmit(async () => { | ||
| const response = await swapCallback?.() | ||
| if (!response) return | ||
|
|
||
| // Set the block containing the response to the oldest valid block to ensure that the |
There was a problem hiding this comment.
shouldn't all these oldest references be newest?
There was a problem hiding this comment.
we want the oldest valid block because that implies that nothing older than that is still valid.
I'm happy to change this to something less ambiguous in a separate PR, but would prefer not to change the naming in this PR.
| } catch (e) { | ||
| // TODO(zzmp): Surface errors from swap. | ||
| console.log(e) | ||
| invariant(trade.trade) |
There was a problem hiding this comment.
is adding an invariant call here the correct move? iirc it throws if the argument is falsy, so this would be an uncaught error and trigger the error boundary.
There was a problem hiding this comment.
onWrap will not be called if wrapType === undefined, but due to the complex type that's not evident to the type checker. We could replace this with a check, but if wrapCallback() returns a valid response, then wrapType should also be valid. I can annotate this with a comment.
| const [, setSwapAmount] = useSwapAmount(Field.INPUT) | ||
| const resetSwapAmount = useCallback(() => setSwapAmount(''), [setSwapAmount]) |
There was a problem hiding this comment.
I was looking for where the the Field.INPUT setting was happening, then realized it's probably happening implicitly here. Just want to point out that this feels a little hard to parse. I don't know if someone reading this code w/o context would be able to recognize that is happening.
There was a problem hiding this comment.
I thought that by semantically naming a derived setter I would make it easier to parse, so this is great feedback. I'll opt for the more explicit setSwapAmount('') inlined, and alias setSwapAmount to setInputAmount or similar.
There was a problem hiding this comment.
specifically I mean it's not immediately clear to me that these lines reset the widget to an exactInput swap if it was not initially in that state. The derived setter makes sense for the Amount for sure, just not the Field/tradeType
| @@ -67,80 +67,82 @@ export default memo(function SwapButton({ disabled }: SwapButtonProps) { | |||
| // Close the review modal on chain change. | |||
| useEffect(() => setOpen(false), [chainId]) | |||
|
|
|||
| const addTransaction = useAddTransaction() | |||
| const addTransactionInfo = useAddTransaction() | |||
There was a problem hiding this comment.
should this hook name change?
There was a problem hiding this comment.
This is adding a TransactionInfo type (Transaction type is different), so I'm going to propagate this change to all callsites.
| const response = await wrapCallback() | ||
| if (!response) return | ||
|
|
||
| invariant(wrapType) |
There was a problem hiding this comment.
This is a subtly different assertion than the one being removed. WrapType.NONE (0) will throw here now. Is that intentional?
There was a problem hiding this comment.
WrapType.NONE was removed in #159 - this is now a type alias for TransactionType.WRAP | TransaactionType.UNWRAP | undefined.
I can (and should) still make this more explicit, so I'll update to wrapType !== undefined.
|
🎉 This PR is included in version 2.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Zeroes out the inputs and sets Field.INPUT as the independent field (ie TradeType.EXACT_INPUT) once a swap or wrap is submitted. This is in line with github.com/Uniswap/interface, as per @infredible.
Screen.Recording.2022-08-23.at.5.55.12.PM.mov