feat(earn): Add earn withdraw bottom sheet #6142

finnian0826 · 2024-10-10T20:03:53Z

Description

Add bottom sheet for different withdraw options, show if >=2 of those options exist for a user.

Test plan

Unit tests added and updated

Manual:

Can partial withdraw:

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-14.at.11.09.25.mp4

Can't partial withdraw:

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-14.at.11.12.22.mp4

Related issues

Fixes ACT-1385

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-10-10T21:26:23Z

Codecov Report

Attention: Patch coverage is 99.08257% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.89%. Comparing base (0f20649) to head (e18782f).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/earn/poolInfoScreen/WithdrawBottomSheet.tsx	98.68%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6142      +/-   ##
==========================================
+ Coverage   88.87%   88.89%   +0.01%     
==========================================
  Files         730      734       +4     
  Lines       31029    31136     +107     
  Branches     5693     5413     -280     
==========================================
+ Hits        27577    27678     +101     
- Misses       3256     3415     +159     
+ Partials      196       43     -153

Files with missing lines	Coverage Δ
src/analytics/Events.tsx	`100.00% <100.00%> (ø)`
src/analytics/Properties.tsx	`100.00% <ø> (ø)`
src/earn/ActionCard.tsx	`100.00% <100.00%> (ø)`
...c/earn/poolInfoScreen/BeforeDepositBottomSheet.tsx	`100.00% <100.00%> (ø)`
src/earn/poolInfoScreen/EarnPoolInfoScreen.tsx	`93.82% <100.00%> (+0.29%)`	⬆️
src/earn/types.ts	`100.00% <ø> (ø)`
src/icons/Exit.tsx	`100.00% <100.00%> (ø)`
src/icons/Trophy.tsx	`100.00% <100.00%> (ø)`
src/earn/poolInfoScreen/WithdrawBottomSheet.tsx	`98.68% <98.68%> (ø)`

... and 71 files 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 0f20649...e18782f. Read the comment docs.

satish-ravi · 2024-10-18T21:40:59Z

src/earn/BeforeDepositBottomSheet.tsx

@@ -25,14 +25,14 @@ import { Spacing } from 'src/styles/styles'
 import { TokenBalance } from 'src/tokens/slice'
 import { getTokenAnalyticsProps } from 'src/tokens/utils'

-function ActionCard({ action }: { action: BeforeDepositAction }) {
+export function ActionCard({ action }: { action: BeforeDepositAction | WithdrawAction }) {


Might be worth moving this into its own file. Maybe even share with other parts of the app (token details, etc), seems like it's something we use throughout the app and would be nice to change styles in a single place for mobile stack apps

satish-ravi · 2024-10-18T21:45:08Z

src/earn/EarnPoolInfoScreen.test.tsx

+    expect(getByTestId('Earn/ActionCard/Claim')).toBeTruthy()
+    expect(getByTestId('Earn/ActionCard/Exit')).toBeTruthy()
+  })
+  it('tapping withdraw on WithdrawBottomSheet navigates to enter amount screen', () => {


this file is getting too big, I've started breaking EarnPoolInfoScreen into multiple files on #6176, haven't broken out tests yet. But would it be possible to have all the bottom sheet specific tests in a WithdrawBottomSheet.test.tsx? and then EarnPoolInfoScreen.test.tsx can have a single test to ensure bottom sheet is opened.

@satish-ravi yeah I can do that, for having the bottom sheet tests in their own file, would I still render the pool info screen and then have it tap to open the bottom sheet, or is there a way to render just the bottoms sheet?

moved to their own file in 5ceadc2, did it similar to EarnDepositBottomSheet.test.tsx where I could just render the bottom sheet and seems to work as expected!

satish-ravi · 2024-10-18T21:51:01Z

src/icons/Claim.tsx

Can we name the icon Trophy? Would be good to keep it in sync with the name in the design system https://www.figma.com/design/erFfzHvSTm5g1sjK6jWyEH/Working-Design-System?node-id=2100-5070&node-type=frame&t=lvQvczdzHRWQBGKn-0

satish-ravi · 2024-10-18T21:55:47Z

src/positions/types.ts

@@ -25,6 +25,11 @@ export interface EarningItem {
  includedInPoolBalance?: boolean
 }

+export enum ClaimType {


can this just be a string literal type instead of an enum?

satish-ravi · 2024-10-18T21:57:47Z

src/analytics/Properties.tsx

@@ -1639,6 +1639,7 @@ interface EarnEventsProperties {
    hasTokensOnSameNetwork: boolean
    hasTokensOnOtherNetworks: boolean
  }
+  [EarnEvents.earn_select_withdraw_type]: { type: 'partialWithdraw' | 'claim' | 'exit' }


nit: keep all the pool_info_tap ones together

satish-ravi · 2024-10-18T21:57:52Z

src/analytics/docs.ts

@@ -609,6 +609,7 @@ export const eventDocs: Record<AnalyticsEventType, string> = {
  [EarnEvents.earn_pool_info_tap_info_icon]: `When the user taps an info icon on the earn pool info screen`,
  [EarnEvents.earn_pool_info_tap_withdraw]: `When the user taps the withdraw button on the pool info screen`,
  [EarnEvents.earn_pool_info_tap_deposit]: `When the user taps the deposit button on the pool info screen`,
+  [EarnEvents.earn_select_withdraw_type]: `When a use selects a withdraw type on the withdraw bottom sheet`,


same as above

src/analytics/Properties.tsx

src/earn/types.ts

src/analytics/docs.ts

Co-authored-by: Tom McGuire <Mcgtom10@gmail.com>

src/earn/types.ts

locales/base/translation.json

Co-authored-by: Tom McGuire <Mcgtom10@gmail.com>

satish-ravi · 2024-10-21T21:19:28Z

src/utils/ActionCard.tsx

we generally don't put components in utils, you can leave this at src/earn. I believe eventually this should go to src/components and reused across the other bottom sheets. Can you make an issue for it?

satish-ravi · 2024-10-21T21:22:11Z

src/earn/poolInfoScreen/WithdrawBottomSheet.tsx

+        {canPartialWithdraw && (
+          <PartialWithdrawAction forwardedRef={forwardedRef} pool={pool} hasRewards={canClaim} />
+        )}


is the gate check needed here? this bottom sheet will never show up if the gate is disabled per changes in EarnPoolInfoScreen right?

I think it is needed, in EarnPoolInfoScreen, we show the bottom sheet if (hasRewards || partialWithdrawalsEnabled), so if the user has rewards but the gate is disabled, then the bottom sheet will still be shown with the options to claim rewards or exit.

satish-ravi · 2024-10-21T21:22:48Z

src/earn/poolInfoScreen/WithdrawBottomSheet.test.tsx

+  },
+})
+
+it('tapping withdraw on WithdrawBottomSheet navigates to enter amount screen', () => {


wrap these in a describe

satish-ravi · 2024-10-21T21:23:40Z

src/earn/poolInfoScreen/WithdrawBottomSheet.test.tsx

+  expect(AppAnalytics.track).toHaveBeenCalledWith(EarnEvents.earn_select_withdraw_type, {
+    type: 'PartialWithdraw',
+  })
+  expect(navigate).toHaveBeenCalledWith(Screens.EarnEnterAmount, {


needs a jest.clearAllMocks in a beforeEach to ensure clean assertions

src/earn/poolInfoScreen/EarnPoolInfoScreen.tsx

satish-ravi · 2024-10-21T21:34:45Z

src/earn/poolInfoScreen/EarnPoolInfoScreen.tsx

+  const rewardsPositions = useSelector(positionsWithBalanceSelector).filter((position) =>
+    pool.dataProps.rewardsPositionIds?.includes(position.positionId)
+  )
+  const hasRewards = useMemo(() => rewardsPositions.length > 0, [rewardsPositions])


can the filter also go inside the useMemo?

satish-ravi · 2024-10-21T21:35:40Z

src/analytics/Properties.tsx

@@ -1642,6 +1647,7 @@ interface EarnEventsProperties {
  [EarnEvents.earn_pool_info_tap_safety_details]: EarnCommonProperties & {
    action: 'expand' | 'collapse'
  }
+  [EarnEvents.earn_select_withdraw_type]: { type: WithdrawActionName }


can we include EarnCommonProperties here?

Missed feature gating

finnian0826 added 3 commits October 10, 2024 14:02

feat(earn): Add earn withdraw bottom sheet

2c91a14

update root state schema

9bae78a

add testId to deposit button to fix test

578ac1a

finnian0826 added 7 commits October 10, 2024 15:29

add extra thing for Allbridge

11cd46d

add unit tests

253d8c4

more unit tests

928b893

lint

0fcf57d

fix claim check

8a15457

Merge branch 'main' into finnian0826/act-1385

1e37e52

updates after merge

3e0e238

finnian0826 marked this pull request as ready for review October 18, 2024 17:00

Merge branch 'main' into finnian0826/act-1385

9d2405c

satish-ravi reviewed Oct 18, 2024

View reviewed changes

MuckT reviewed Oct 19, 2024

View reviewed changes

src/analytics/Properties.tsx Outdated Show resolved Hide resolved

src/earn/types.ts Outdated Show resolved Hide resolved

finnian0826 added 8 commits October 21, 2024 09:06

feedback pt 1

57a3ced

extra

7664636

feedback pt 2

fd7fce8

move to folder

a8a0c8d

Merge branch 'main' into finnian0826/act-1385

b32b43e

fix merge

5a96474

cleanup

15ed739

feedback pt 3

5ceadc2

MuckT reviewed Oct 21, 2024

View reviewed changes

src/analytics/docs.ts Outdated Show resolved Hide resolved

Update src/analytics/docs.ts

b09ab83

Co-authored-by: Tom McGuire <Mcgtom10@gmail.com>

MuckT reviewed Oct 21, 2024

View reviewed changes

src/earn/types.ts Outdated Show resolved Hide resolved

tom feedback

3f4abb2

MuckT previously approved these changes Oct 21, 2024

View reviewed changes

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

Update locales/base/translation.json

7865012

Co-authored-by: Tom McGuire <Mcgtom10@gmail.com>

satish-ravi reviewed Oct 21, 2024

View reviewed changes

MuckT self-requested a review October 21, 2024 22:36

finnian0826 added 2 commits October 22, 2024 10:12

feedback

9efe91b

feedback

c5df7b0

MuckT approved these changes Oct 22, 2024

View reviewed changes

finnian0826 added 2 commits October 22, 2024 12:51

cleanup comment

a3f236c

copy update

e18782f

finnian0826 enabled auto-merge October 22, 2024 19:25

finnian0826 added this pull request to the merge queue Oct 22, 2024

Merged via the queue into main with commit a1f5862 Oct 22, 2024
15 checks passed

finnian0826 deleted the finnian0826/act-1385 branch October 22, 2024 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(earn): Add earn withdraw bottom sheet #6142

feat(earn): Add earn withdraw bottom sheet #6142

finnian0826 commented Oct 10, 2024 •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading

satish-ravi Oct 18, 2024

satish-ravi Oct 18, 2024

finnian0826 Oct 21, 2024

finnian0826 Oct 21, 2024

satish-ravi Oct 18, 2024

satish-ravi Oct 18, 2024

satish-ravi Oct 18, 2024

satish-ravi Oct 18, 2024

satish-ravi Oct 21, 2024

satish-ravi Oct 21, 2024

finnian0826 Oct 22, 2024

satish-ravi Oct 21, 2024

satish-ravi Oct 21, 2024

satish-ravi Oct 21, 2024

satish-ravi Oct 21, 2024

feat(earn): Add earn withdraw bottom sheet #6142

feat(earn): Add earn withdraw bottom sheet #6142

Conversation

finnian0826 commented Oct 10, 2024 • edited Loading

Description

Test plan

Related issues

Backwards compatibility

Network scalability

codecov bot commented Oct 10, 2024 • edited Loading

Codecov Report

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

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

Choose a reason for hiding this comment

finnian0826 commented Oct 10, 2024 •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading