feat(swap): Initial Swap screen redesign #5409

jophish · 2024-05-13T21:05:30Z

Description

First part of RET-1065. Updates the design of swap screen to match the designs in Figma. The rest of the behavior outlined in the ticket will come in followup PRs.

Test plan

Unit and manual tested. See video below:

swap-redesign-2024-05-13_17.03.19.mp4

Related issues

Part of RET-1065.

Backwards compatibility

Yes.

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

codecov · 2024-05-13T21:23:28Z

Codecov Report

Attention: Patch coverage is 89.47368% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 86.31%. Comparing base (5946f19) to head (6851300).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5409   +/-   ##
=======================================
  Coverage   86.31%   86.31%           
=======================================
  Files         756      756           
  Lines       31185    31205   +20     
  Branches     5347     5353    +6     
=======================================
+ Hits        26916    26935   +19     
- Misses       4036     4037    +1     
  Partials      233      233

Files	Coverage Δ
src/analytics/Events.tsx	`100.00% <100.00%> (ø)`
src/analytics/Properties.tsx	`100.00% <ø> (ø)`
src/components/Touchable.tsx	`100.00% <100.00%> (ø)`
src/icons/DownIndicator.tsx	`100.00% <100.00%> (ø)`
src/swap/SwapScreen.tsx	`93.78% <100.00%> (+0.09%)`	⬆️
src/swap/SwapAmountInput.tsx	`91.66% <83.78%> (+0.62%)`	⬆️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5946f19...6851300. Read the comment docs.

jeanregisser

Great!

Things/questions I noted from the video:

Does the token selection also work when tapping the container of the down arrow?
The right padding/margin in the skeleton loader seems different from the left one. It should be the same.
The ripple effect on the switch token button is being clipped by containers above and below it. It should be above them.
I'm slightly concerned by the fact that the "MAX" button can change position based on the length of the $ value. Maybe something we can improve with Kayla?

locales/base/translation.json

src/swap/SwapScreen.tsx

bakoushin · 2024-05-14T09:44:13Z

Looking great!

Noticed a few rendering issues:

The switch token button is rendered behind the "to" field on the iOS emulator (see screenshot):

If we put a really long number, everything breaks (see video). This could possibly be a real-life issue for fiats having really large denominations.
Perhaps we could stop the fiat value from growing in width past 40% of the parent width?

Simulator.Screen.Recording.-.iPhone.15.-.2024-05-14.at.11.30.35.mp4

+1 to Jean:

I would expect the whole top part of the swap amount box to be pressable. Not only the arrow button.
I wonder if instead of ripple across the switch token button, we can add a rotation animation instead? Like, the arrow after a tap rotates 360 degrees (don't know though if it is possible to turn off the ripple effect).

jophish · 2024-05-14T20:09:38Z

Responding to some comments:

@jeanregisser

Does the token selection also work when tapping the container of the down arrow?

It did not, but now it does - this required a bit of rejiggering to how Touchables work, since the border radius is uneven depending on the state of the screen.

The right padding/margin in the skeleton loader seems different from the left one. It should be the same.

Thanks! This has been fixed.

The ripple effect on the switch token button is being clipped by containers above and below it. It should be above them.

This was a tricky one. I'm somewhat sure that we were running up against this bug, since I did not encounter the adverse overlapping behavior when using PlatformTouchable.SelectableBackground() as the background. Fortunately, I was able to work around this using the foreground option instead of background as described here.

I'm slightly concerned by the fact that the "MAX" button can change position based on the length of the $ value. Maybe something we can improve with Kayla?

The designs have been updated to move the "MAX" button to the right of the fiat estimation.

@bakoushin

The switch token button is rendered behind the "to" field on the iOS emulator (see screenshot):

Would you mind testing on iOS again? This is working on Android, but made a few small changes.

If we put a really long number, everything breaks (see video). This could possibly be a real-life issue for fiats having really large denominations.

I've raised this with Kayla, waiting to hear back about design ideas.

I would expect the whole top part of the swap amount box to be pressable. Not only the arrow button.

Addressed above!

I wonder if instead of ripple across the switch token button, we can add a rotation animation instead?

I suppose we could do this, though if the ripple is working correctly (which it should be, now) we could also stick with that.

See new video here:

swap-redesign-2-2024-05-14_16.08.42.mp4

…jophish/ret-1065

kathaypacific

looking great!

locales/base/translation.json

src/analytics/docs.ts

src/swap/SwapScreen.tsx

kathaypacific · 2024-05-17T09:16:15Z

src/components/Touchable.tsx

+ <PlatformTouchable
+ {...passThroughProps}
+ background={useForeground ? undefined : background}
+ foreground={useForeground ? background : undefined}


i got really hung up on this even though it's not important because it's not easy to understand what useForeground means. (also reading if useForeground, use background is weird). it seems like this prop should be used if the Touchable has any images as children and you want the ripple to be rendered above the image - so perhaps a more intuitive prop for this component could be shouldRenderRippleAbove or rippleAboveImages or hasImageChildren?

Yeah, agreed that it's weird as-is... I like shouldRenderRippleAbove, since it does have utility outside of using images within the Touchable.

src/components/Touchable.tsx

src/swap/SwapAmountInput.tsx

kathaypacific · 2024-05-17T09:29:15Z

src/swap/SwapAmountInput.tsx

- ...fontStyles.xsmall,
- fontWeight: 'bold',
+ tokenInfo: {
+ flexDirection: 'row',


i think you need an alignItems center here so that the icon is vertically centered. if you increase your device font size, the icon is no longer centered currently

bump on this! :)

src/swap/SwapScreen.tsx

kathaypacific · 2024-05-17T09:52:29Z

src/swap/SwapAmountInput.tsx

 },
 fiatValue: {
 ...typeScale.bodyXSmall,
+ paddingLeft: Spacing.Smallest8,
+ maxWidth: '40%',


if we're adding a max width, we should also add a numberOfLines={1} so that the text is ellipsised when the limit is hit rather than spreading to new lines (e.g. if you type a long number like 123455678923624856 in the token amount - unlikely to happen but if the device font size is large then this number can become smaller to reach the limit)

Aha, good catch.. I had tried to do this but was setting numberOfLines={1} at the wrong layer and it wasn't working as intended. Setting it on the SwapAmountInput/FiatValue component works well!

kathaypacific

looks great!! 💅🏼

kathaypacific · 2024-05-21T13:11:39Z

src/swap/SwapScreen.test.tsx

+ expect(within(swapFromContainer).getByText('CELO')).toBeTruthy()
+
+ expect(within(swapToContainer).getByTestId('SwapAmountInput/Input')).toBeTruthy()
+ expect(within(swapToContainer).getByText('cUSD')).toBeTruthy()


nit: selectSwapTokens already includes these assertions so you don't need to do it again here :)

kathaypacific · 2024-05-21T13:13:46Z

src/swap/SwapAmountInput.tsx

- ...fontStyles.xsmall,
- fontWeight: 'bold',
+ tokenInfo: {
+ flexDirection: 'row',


bump on this! :)

jophish and others added 2 commits May 13, 2024 17:02

Initial changes for Swap redesign

d8f41c3

Merge branch 'main' into jophish/ret-1065

6318608

Merge branch 'main' into jophish/ret-1065

f5083e7

jeanregisser reviewed May 14, 2024

View reviewed changes

locales/base/translation.json Outdated Show resolved Hide resolved

src/swap/SwapScreen.tsx Outdated Show resolved Hide resolved

bakoushin reviewed May 14, 2024

View reviewed changes

src/swap/SwapScreen.tsx Show resolved Hide resolved

jophish and others added 8 commits May 14, 2024 16:22

Respond to comments

7a769e8

Merge branch 'jophish/ret-1065' of github.com:valora-inc/wallet into …

9866c10

…jophish/ret-1065

Merge branch 'main' into jophish/ret-1065

db30579

Remove unused style

cd5b22e

Merge branch 'jophish/ret-1065' of github.com:valora-inc/wallet into …

17e7db7

…jophish/ret-1065

Limit fiat value section width

09f9b33

Merge branch 'main' into jophish/ret-1065

9138ac2

Merge branch 'main' into jophish/ret-1065

68668ff

kathaypacific reviewed May 17, 2024

View reviewed changes

jophish and others added 2 commits May 20, 2024 10:56

Merge branch 'main' into jophish/ret-1065

bdb0d91

Changes

4555251

kathaypacific approved these changes May 21, 2024

View reviewed changes

jophish and others added 2 commits May 21, 2024 12:04

Comments

7a95688

Merge branch 'main' into jophish/ret-1065

6851300

jophish added this pull request to the merge queue May 21, 2024

Merged via the queue into main with commit ea65cbc May 21, 2024
16 checks passed

jophish deleted the jophish/ret-1065 branch May 21, 2024 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(swap): Initial Swap screen redesign #5409

feat(swap): Initial Swap screen redesign #5409

jophish commented May 13, 2024 •

edited

codecov bot commented May 13, 2024 •

edited

jeanregisser left a comment

bakoushin commented May 14, 2024

jophish commented May 14, 2024 •

edited

kathaypacific left a comment

kathaypacific May 17, 2024

jophish May 20, 2024

kathaypacific May 17, 2024

kathaypacific May 21, 2024

kathaypacific May 17, 2024

jophish May 20, 2024

kathaypacific left a comment

kathaypacific May 21, 2024

kathaypacific May 21, 2024

feat(swap): Initial Swap screen redesign #5409

feat(swap): Initial Swap screen redesign #5409

Conversation

jophish commented May 13, 2024 • edited

Description

Test plan

Related issues

Backwards compatibility

Network scalability

codecov bot commented May 13, 2024 • edited

Codecov Report

jeanregisser left a comment

Choose a reason for hiding this comment

bakoushin commented May 14, 2024

jophish commented May 14, 2024 • edited

kathaypacific left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kathaypacific left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jophish commented May 13, 2024 •

edited

codecov bot commented May 13, 2024 •

edited

jophish commented May 14, 2024 •

edited