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

added warning for unknown price impact #5597

Merged
merged 6 commits into from Apr 9, 2024
Merged

Conversation

dereknelson
Copy link
Contributor

@dereknelson dereknelson commented Apr 4, 2024

Fixes APP-1295

What changed (plus any additional context for devs)

it appears that when we have no pricing data, the price impact always severe at 100%
if we haven't gotten the price impact response yet, it's NaN

Screen recordings / screenshots

image
pending final copy from @christianbaroni
image

What to test

that this message is not shown when we do have price impact data

Copy link

linear bot commented Apr 4, 2024

@nickbytes
Copy link
Contributor

@dereknelson just to be clear on requirements being met here:

  • if we don't know the price impact, we show the message "Price impact unknown"
  • if we do know the price impact, we show the price impact.

Is that correct?

@dereknelson
Copy link
Contributor Author

dereknelson commented Apr 4, 2024

@nickbytes yeah, in all the cases i tried it seems like the impact defaults to 100% when we don't know it, and is only NaN when it's loading

Copy link
Contributor

@skylarbarrera skylarbarrera left a comment

Choose a reason for hiding this comment

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

lgtm

@dereknelson
Copy link
Contributor Author

@skylarbarrera i'll merge after i add warning to review sheet

@dereknelson dereknelson merged commit cfae287 into develop Apr 9, 2024
4 checks passed
@dereknelson dereknelson deleted the @derek/slippage branch April 9, 2024 17:18
BrodyHughes added a commit that referenced this pull request Apr 10, 2024
…e-changes

* 'develop' of github.com:rainbow-me/rainbow: (44 commits)
  allow open in new tab (#5610)
  added warning for unknown price impact (#5597)
  fix cloudflare protection (#5609)
  improve type checking on web preferences (#5607)
  fix scrolltoindex firing on last card dismissal (#5606)
  make account network switcher work (#5604)
  Dapp browser fixes (#5596)
  Fix close tab btn (#5598)
  browser: add account context menu (#5603)
  Fix instant screenshot setting (#5602)
  swaps: bump sdk (#5583)
  Browser: tab transitions, state update queue (#5582)
  audit: undici (#5594)
  [SWAPS V2]: Add token search logic and ability to select assets (#5547)
  swaps v2 gas (#5526)
  tx sim: other natives (#5585)
  Brody/bump 1.9.21 3 (#5588)
  fix sheet bg (#5590)
  Only hold the active tab ref in BrowserContext (#5579)
  Dapp browser: disable tab closing for empty state (#5573)
  ...
BrodyHughes added a commit that referenced this pull request Apr 10, 2024
* 'develop' of github.com:rainbow-me/rainbow:
  allow open in new tab (#5610)
  added warning for unknown price impact (#5597)
  fix cloudflare protection (#5609)
  improve type checking on web preferences (#5607)
  fix scrolltoindex firing on last card dismissal (#5606)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants