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

♻️ Extract wallet to context #193

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

quentin-burg
Copy link
Collaborator

  • Extract BeaconWallet into Context
  • Extract TezosToolKit into Context
  • Rewrite Context to manage App State and Dispatch function

TODO : Problem with fetching proposals ... To be continued ...

Copy link
Collaborator

@Pilou97 Pilou97 left a comment

Choose a reason for hiding this comment

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

First review

components/LoginModal.tsx Show resolved Hide resolved
components/LoginModal.tsx Show resolved Hide resolved
@@ -77,13 +81,13 @@ const LoginModal = ({ data, onEnd }: { data: string; onEnd: () => void }) => {
setError((e as Error).message);
setCurrentState(State.ERROR);
}
}, [data, state.p2pClient, state.attemptedInitialLogin]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why state.attemptedInitialLogin is no more required?

components/textInputWithComplete.tsx Show resolved Hide resolved
context/state.tsx Show resolved Hide resolved
context/wallet.tsx Outdated Show resolved Hide resolved
context/wallet.tsx Show resolved Hide resolved
context/wallet.tsx Show resolved Hide resolved
Copy link

cloudflare-workers-and-pages bot commented Apr 22, 2024

Deploying tzsafe-ui-ghostnet with  Cloudflare Pages  Cloudflare Pages

Latest commit: e7811ab
Status: ✅  Deploy successful!
Preview URL: https://32076319.tzsafe-ui.pages.dev
Branch Preview URL: https://extract-wallet-to-context.tzsafe-ui.pages.dev

View logs

@quentin-burg quentin-burg marked this pull request as ready for review April 26, 2024 06:34
Copy link
Collaborator

@Pilou97 Pilou97 left a comment

Choose a reason for hiding this comment

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

Cool to have update the package.json and airgap

case "p2pConnect": {
return { ...state, p2pClient: action.payload };
}
case "addDapp": {
if (!state.address) return state;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The address is always defined? I don't know what is addDapp but can it be triggered when the user is disconnected without any safe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

address is no longer defined in the global state but in wallet context.
addDapp allows to connect an external dapp with Tzsafe and user have to import a contract to access to this feature, so I believe disconnected user cannot trigger this action

@@ -401,7 +401,7 @@ export function tezosDomains(
action: BUY_ADDRESS.name,
description: (
<p>
TzSafe doesn't support the entrypoint:{" "}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need spaces anymore? I am wondering if we can use template string 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes you're right :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated all of occurrences of this string

@@ -39,7 +39,7 @@ const test_suit = (setTezosToolkit: (tezos: TezosToolkit) => TezosToolkit) =>
expect(storage.proposal_counter.isEqualTo(BigNumber(0))).toBe(true);
expect(storage.owners).toEqual([owner]);
storage.metadata.get("").then((value: string) => {
expect(value).toEqual(char2Bytes(ipfs_file));
expect(value).toEqual(stringToBytes(ipfs_file));
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the difference between char2Bytes and stringToBytes ? Why do we need to change it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

charToBytes and bytesToChar are deprecated => https://taquito.io/typedoc/functions/_taquito_utils.char2Bytes

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

Successfully merging this pull request may close these issues.

2 participants