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

refactor: Remove useArbTokenBridge from store 2/3 #2089

Open
wants to merge 1 commit into
base: fs-1001/add-useBridgeTokensStore
Choose a base branch
from

Conversation

chrstph-dvx
Copy link
Contributor

@chrstph-dvx chrstph-dvx commented Nov 19, 2024

Summary

Wrap methods with useCallbacks in useArbTokenBridge

Steps to test

@chrstph-dvx chrstph-dvx self-assigned this Nov 19, 2024
@cla-bot cla-bot bot added the cla-signed label Nov 19, 2024
Copy link

vercel bot commented Nov 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Dec 9, 2024 6:30pm

@chrstph-dvx chrstph-dvx changed the title refactor: Remove useArbTokenBridge from store 2/3 - Wrap methods with useCallbacks in useArbTokenBridge refactor: Remove useArbTokenBridge from store 2/3 Nov 19, 2024
@chrstph-dvx chrstph-dvx force-pushed the fs-1001/add-usecallbacks branch from c72b3a9 to 5789015 Compare November 19, 2024 20:15
@chrstph-dvx chrstph-dvx force-pushed the fs-1001/add-useBridgeTokensStore branch from cb9121a to 747c8f4 Compare November 19, 2024 20:15
Copy link
Member

@fionnachan fionnachan left a comment

Choose a reason for hiding this comment

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

other than that one small comment, everything else looks good

@@ -417,130 +451,157 @@ export const useArbTokenBridge = (
]
)
Copy link
Member

Choose a reason for hiding this comment

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

there's a missing dependency here, i know you didn't change it but can you add destinationAddress to the dep array please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in a376a1a (#2089)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants