web icon indicating copy to clipboard operation
web copied to clipboard

Complete WarningAcknowledgement implementation

Open 0xApotheosis opened this issue 10 months ago • 5 comments

Overview

A follow-up to https://github.com/shapeshift/web/pull/6647, we need to add the new WarningAcknowledgement component to the following user flows:

  • [ ] 1. Multi-hop trade, may end up with different asset if something goes wrong
  • [ ] 2. LiFi single-hop trades with intermediary assets

References and additional details

Use the existing implementation as a guide. We'll want to replace the cases noted in https://github.com/shapeshift/web/issues/6605 and https://www.figma.com/file/ZK7L1l38Kzc4SlKekbPnm0/Modals?type=design&node-id=1-250&mode=design&t=MJPI055hYriNnDxz-0 with the new component for a more consistent user experience.

Acceptance Criteria

The WarningAcknowledgement component is in place for each of the cases above.

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response

0xApotheosis avatar Apr 18 '24 02:04 0xApotheosis

Adding to groomed, as it's a completion of the existing groomed task.

0xApotheosis avatar Apr 18 '24 02:04 0xApotheosis

Note to self: grep .dangerousWithdrawWarning and remove those cases.

0xApotheosis avatar Apr 18 '24 21:04 0xApotheosis

@0xApotheosis is 1. being checked correct? As far as I can tell, savers is still using the previous dangerous withdraw div.

With that being said, I wasn't able to recreate the dangerous withdraw logic, as minimum amounts seem to gracefully catch it, though I don't have a BTC saver deposit, which could be the affected one.

gomesalexandre avatar Jun 11 '24 14:06 gomesalexandre

Note to self: grep .dangerousWithdrawWarning and remove those cases.

@0xApotheosis finally managed to repro withdraws on BTC despite current fees endpoint shenanigans and it looks like indeed, we can remove these, since the minimum effectively catches dangerous withdraws, meaning we don't need neither the current dangerousWithdrawWarning nor the new ack?

Image

Image

Image

gomesalexandre avatar Jun 11 '24 21:06 gomesalexandre

You are absolutely right @gomesalexandre! I've removed that line item from the ticked, as I think it was actually meant to reference LPs.

0xApotheosis avatar Jun 12 '24 06:06 0xApotheosis