Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent unsafe THOR flow #6605

Closed
gomesalexandre opened this issue Mar 28, 2024 · 3 comments
Closed

Consistent unsafe THOR flow #6605

gomesalexandre opened this issue Mar 28, 2024 · 3 comments
Assignees
Labels
needs product Requires product input before bounty thorchain

Comments

@gomesalexandre
Copy link
Contributor

Overview

Currently, the flow for "depositing/withdrawing" (i.e withdrawing from savers vault, borrowing into lending, adding liquidity in LP, or swapping) has some notion of an unsafe deposit, which is very similar, though slightly different per domain, and with rough edges:

  • In Lending, an unsafe deposit is a deposit with slippage higher than the current 5% threshold
image
  • In LP, an unsafe deposit is a deposit with slippage higher than the current 5% threshold
image
  • In Swapper, an unsafe swap is a swap that's lower than the recommended THOR minimum
image
  • In savers, an unsafe withdraw is a withdraw that's lower or equal to protocol fees - slightly unrelated in that this is a different component/design altogether, and you cannot acknowledge it, it is disabled without the ability to continue
image

There are a few issues with this:

  • code duplication
  • slightly different UXs re: having to acknowledge the warning vs. the action being disabled altogether
  • most importantly, this warning/acknowledgement superseeds any other error state, which means in case of e.g deposits disabled, or any other error a la insufficient funds, the user would first have to acknowledge the unsafe action before seeing the error

References and additional details

#6600 (comment) initially raised here

Acceptance Criteria

Product

  • We have a consistent product flow and design for the concept of unsafe action across the app
  • We know how to handle multiple unsafe actions if that's ever a concern

Engineering - TBD after we refine the flow

  • We do not superseed other errors states with the unsafe acknowledgement flow, and only render it if there are no other errors
  • Copies are passed programmatically and this is not THORChain-specific, being able to be reused if needed

Nice to have:

  • We have a higher-order-componentwrapping a button or button-like component and doing all the warning/acknowledgement logic, as well as the logic of only being rendered if there are no other errors (e.g a prop a la isDisabled or <RenderIf /> HOC of sorts) so that consumers are now only concerned about consuming this component, without duplicating the unsafe ack logic

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response

@0xean
Copy link
Contributor

0xean commented Apr 1, 2024

@twblack88 - please do the things

@0xean 0xean removed the needs engineering Requires engineering input before bounty label Apr 1, 2024
@twblack88
Copy link
Contributor

Spiked on this with @0xApotheosis https://www.figma.com/file/ZK7L1l38Kzc4SlKekbPnm0/Modals?type=design&node-id=1%3A250&mode=design&t=2Q6UeUWcj21JCvIm-1

We're gonna flesh out the states, what we expect, and what should happen from there so we have standardization and we also unblock #6577 as a result.

@twblack88
Copy link
Contributor

twblack88 commented Apr 5, 2024

Follow on for @gomesalexandre notes For ENG (@0xApotheosis monday):
We have an overlay component handling the warning/acknowledgement logic (programmatic, renders the same copies as currently)
Screenshot 2024-04-05 at 1 57 22 PM

  • (e.g a prop isDisabled or <RenderIf /> HOC of sorts) so that consumers are now only concerned about
    consuming this component, without duplicating the unsafe ack logic overlays. when you lcik
  • Component will have a header and a body component, this should passed through the parent.
  • overlay will be triggered on the preview, borrow, repay, add liquidity, remove liquidity, continue buttons
  • we don’t want to supercede other error states not an issue anymore - since we will only display this when clicking continue (meaning if we're unable to click continue because of any other error, we'll never see this)
  • wrapper has absolute position, dims background. (can leave the animating to @reallybeard )
  • reference the <MessageOverlay /> keplr blurry swapper component for how this could be done.
  • When the user clicks I understand, we then trigger the usual onClick behavior and dismiss the overlay.
  • User can cancel, this will close the overlay and no onClick is fired.
  • Stretch animate this overlay from the bottom using framer motion

@0xApotheosis 0xApotheosis self-assigned this Apr 8, 2024
@0xApotheosis 0xApotheosis moved this from Backlog to In progress in ShapeShift Dashboard Apr 8, 2024
@0xean 0xean removed the needs product Requires product input before bounty label Apr 8, 2024
@0xean 0xean added the needs product Requires product input before bounty label May 6, 2024
@0xean 0xean assigned twblack88 and unassigned 0xApotheosis May 6, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in ShapeShift Dashboard Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs product Requires product input before bounty thorchain
Projects
Status: Done
Development

No branches or pull requests

5 participants