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

Swaps: fix crashes, logic and UI fixes #5839

Merged
merged 8 commits into from
Jun 12, 2024

Conversation

christianbaroni
Copy link
Member

@christianbaroni christianbaroni commented Jun 12, 2024

Fixes APP-1538
Fixes APP-1552
Fixes APP-1582
Fixes APP-1583
Fixes APP-1591
Fixes RNBW-4755

What changed (plus any additional context for devs)

  • Fixes division by zero crashes
  • Fixes exchange rate bubble button presses
  • Removes hardcoded decimal places from stablecoin input amounts
  • Fixes NaN issues (the toLocaleString changes are temporary and will be superseded by Ben's changes)
  • Improves buy list data memoization (removes an unnecessary extra useMemo)
  • Adds PIXEL_RATIO to deviceUtils
  • Allows flipping when only one asset is selected and speeds up flipping (no longer using setAsset which adds a bit of duplicate code but prevents complicating setAsset
  • Adjusts buy list sorting to ensure the chain's native asset is listed first
  • Fixes buy list empty state layout
  • Improves performance of the animated input styles, specifically the height style

Screen recordings / screenshots

What to test

@christianbaroni christianbaroni changed the title Swaps: fix crashes, logic, and UI fixes Swaps: fix crashes, logic and UI fixes Jun 12, 2024
Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

Haven't ran it yet, just did a mobile run through of the code and some small questions/nitpicks.


// This causes a heavy re-render in the output token list, so we delay updating the selected output chain until
// the animation is most likely complete.
chainSetTimeoutId.current = setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Smart fix, nice

Comment on lines +171 to +173
const nativeValue = `${Number(inputValues.value.inputNativeValue).toLocaleString('en-US', {
currency: 'USD',
style: 'currency',
Copy link
Contributor

Choose a reason for hiding this comment

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

This will conflict with Ben's PR afaik

Comment on lines +213 to +215
const nativeValue = `${Number(inputValues.value.outputNativeValue).toLocaleString('en-US', {
currency: 'USD',
style: 'currency',
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

src/__swaps__/utils/swaps.ts Show resolved Hide resolved
Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

Code LGTM!

src/__swaps__/utils/swaps.ts Show resolved Hide resolved
@christianbaroni christianbaroni merged commit 23f911c into develop Jun 12, 2024
6 checks passed
@christianbaroni christianbaroni deleted the @christian/swaps-logic-and-ui-fixes branch June 12, 2024 23:19
BrodyHughes added a commit that referenced this pull request Jun 13, 2024
…bow into brody/lint-on-pre-commit

* 'brody/lint-on-pre-commit' of github.com:rainbow-me/rainbow:
  Math related fixes (#5837)
  Hide Flashbots outside of mainnet (#5842)
  fix getProviderForNetwork type (#5843)
  Fix isSwapping not being reset (#5840)
  Swaps: fix crashes, logic and UI fixes (#5839)
  default to fast suggestion if custom is not set (#5841)
  guard each entry point into swaps to prevent readonly wallets from accessing it (#5835)
  Swaps: token list and search improvements (#5834)
  Add analytics (#5805)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants