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

feat: refactor onboarding #478

Merged

Conversation

mohandast52
Copy link
Collaborator

@mohandast52 mohandast52 commented Nov 21, 2024

Proposed changes

  • Onboarding flow

(The app is running after commenting out a lot of code.)

Screenshot

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@mohandast52 mohandast52 added the enhancement New feature or request label Nov 21, 2024
@mohandast52 mohandast52 self-assigned this Nov 21, 2024
Comment on lines 65 to 67
const master = masterSafes.find(
(safe) => safe.owner === WalletOwnerType.Master,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

masterSafes is pre-filtered

Suggested change
const master = masterSafes.find(
(safe) => safe.owner === WalletOwnerType.Master,
);
const master = masterSafes[0];

@mohandast52 mohandast52 marked this pull request as ready for review November 21, 2024 19:03
Co-authored-by: Josh Miller <31908788+truemiller@users.noreply.github.com>
},
};

export const SetupEoaFunding = () => {
const { goto } = useSetup();
const { masterEoaAddress } = useWallet();
const { masterEoa } = useMasterWalletContext();
const { walletBalances } = useBalanceContext();
Copy link
Collaborator

@truemiller truemiller Nov 21, 2024

Choose a reason for hiding this comment

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

can get masterWalletBalances from useMasterBalances()
not a major issue for now though, this works fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice, updated :)

mohandast52 and others added 2 commits November 22, 2024 00:37
Co-authored-by: Josh Miller <31908788+truemiller@users.noreply.github.com>
Co-authored-by: Josh Miller <31908788+truemiller@users.noreply.github.com>
@@ -24,14 +25,18 @@ export const MECHS: Mechs = {
name: 'Agent Mech',
contract: new MulticallContract(
'0x77af31De935740567Cf4fF1986D04B2c964A786a',
AGENT_MECH_ABI,
AGENT_MECH_ABI.filter(
Copy link
Collaborator

@truemiller truemiller Nov 21, 2024

Choose a reason for hiding this comment

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

probably better to put on the ABI itself, but this is fine for now; no need for JsonFragment either, can use Abi from types

no 100%, but i don't think we'll ever need types like constructor, or event, for example 🤔

@@ -225,6 +234,12 @@ export const SetupEoaFunding = () => {
goto(SetupScreen.SetupCreateSafe);
}, 5000);

const eoaBalance = walletBalances.find((e) => e.isNative);
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we check here chainId as well? like the currentChain or maybe even homeChainId from selectedAgentConfig?

  • probably can use useMasterBalances instead

frontend/components/SetupPage/Create/SetupEoaFunding.tsx Outdated Show resolved Hide resolved
frontend/components/SetupPage/SetupWelcome.tsx Outdated Show resolved Hide resolved
frontend/pages/_app.tsx Outdated Show resolved Hide resolved
frontend/pages/_app.tsx Outdated Show resolved Hide resolved
@mohandast52 mohandast52 merged commit 4c80e72 into refactor/multi-chain-support-frontend Nov 21, 2024
2 of 4 checks passed
@mohandast52 mohandast52 deleted the mohan/refactor-onboarding branch November 21, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants