feat: add HighValueLoss emitter event #86
Conversation
| emitter.emit(WidgetEvent.RouteHighValueLoss, { | ||
| fromAmountUsd: route.fromAmountUSD, | ||
| gasCostUSD: route.gasCostUSD, | ||
| toAmountUSD: route.toAmountUSD, | ||
| valueLoss: Big(route.toAmountUSD || 0) | ||
| .div(Big(route.fromAmountUSD || 0).plus(Big(route.gasCostUSD || 0))) | ||
| .minus(1) | ||
| .mul(100) | ||
| .round(2, Big.roundUp) | ||
| .toString(), | ||
| }); |
There was a problem hiding this comment.
We can't just emit events in the middle of the render phase. This should be wrapped in useEffect. Also, later on, if we decide bottom sheet to stay mounted all the time this can cause issues of firing event even when we don't show the bottom sheet to the user.
I think the best place to fire this event is after we call it to open:
Wdyt? 🙂
There was a problem hiding this comment.
@chybisov Thanks for wrapping your head around this and finding that bug! 👍
The approach you pointed out was a good idea on the issue which I was focussing on first: Triggering the Emit once the HighValueLoss-message pops up.
Now, as I continued to work on, I thought: The best would be to emit an event whenever a user actually continues: That will be useful for later tracking.
Having that message emit is not a bad thing, but what really matters is when the user continues.
What do you think of that approach? Sure, I will return to your solution if you prefer!
There was a problem hiding this comment.
Yes, Continue is also a great place to emit this event. I thought about this too, but I also thought maybe we want to track precisely the event of showing the bottom sheet 😅
Ticket: https://lifi.atlassian.net/browse/LF-2817