Skip to content
This repository has been archived by the owner on Dec 20, 2022. It is now read-only.

Removed torus wallet connection #135

Merged
merged 7 commits into from
Nov 2, 2021
Merged

Removed torus wallet connection #135

merged 7 commits into from
Nov 2, 2021

Conversation

waylandli
Copy link
Contributor

@waylandli waylandli commented Oct 25, 2021

Purpose

https://www.pivotaltracker.com/n/projects/2436354/stories/180031983

We wanted to delete functionality for torus connection as we want to develop the client using metamask for now.

Change summary

  • Looked for torus files and deleted them
  • Deleted mentions of options that allow torus
  • deleted test cases of torus functionality
  • You can connect the wallet through one click rather than clicking through the modal
  • Removed log in modal
  • Simplified some wallet code

To use the Example Client, you must connect with either MetaMask or
Torus.
</p>
<p>To use the Example Client, you must connect with MetaMask</p>
<Tabs defaultActiveKey="1">
<Tabs.TabPane tab="MetaMask" key="1">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to have the tabs anymore since there is no alternate option?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree, get rid of the tabs.

@@ -22,8 +20,6 @@ export const wallet = (walletType: WalletType): Wallet => {
switch (walletType) {
case WalletType.NONE:
Copy link
Collaborator

Choose a reason for hiding this comment

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

How do we want this to work now? Just thinking about if it is the example client's responsibility to have the option of other wallets, or if we just start deleting code like this and simplify the code to is logged into metamask or no.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we just start deleting code like this and simplify the code to is logged into metamask or no.

IMO this is the better idea - it will simplify the code and the UI. We need to always keep in mind that this is an example cilent, showing devs how to implement for DSNP, and expect that people will fork it if they want to make changes.

Comment on lines -49 to -55
<div>OR</div>
<button
className="LoginModal__loginTorus"
onClick={() => setWalletType(wallet.WalletType.TORUS)}
>
Torus
</button>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After removing the torus button, The user has to press connect then metamask when metamask is the only choice. Should we remove the need for two clicks to connect to metamask?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Changed my mind, yea I'd say nuke it, make only 1 click

Copy link
Collaborator

@claireolmstead claireolmstead left a comment

Choose a reason for hiding this comment

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

🥇

@waylandli waylandli merged commit a9bf658 into main Nov 2, 2021
@waylandli waylandli deleted the chore/torus/180031983 branch November 2, 2021 16:20
This was referenced Nov 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants