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

Add CeloDance Wallet to Celo use-contractkit #33

Closed
wants to merge 1 commit into from

Conversation

zhang-hongtao
Copy link
Contributor

as a celo mobile wallet, celodance support the transfer and staking of celo assets. after talking with clabs and moola, we propose to integrate celodance wallet to moola. the development work is all finished, so please merge the code, thanks!

as a celo mobile wallet, celodance support the transfer and staking of celo assets. after talking with clabs and moola, we propose to integrate celodance wallet to moola. the development work is all finished, so please merge the code, thanks!
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please see the comments below

@@ -72,6 +72,34 @@
"@babel/helper-validator-identifier" "^7.14.9"
to-fast-properties "^2.0.0"

"@celo-tools/use-contractkit@1.1.0":
Copy link
Contributor

Choose a reason for hiding this comment

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

Why has the yarn.lock file changed?

[SupportedProviders.CeloDance]: {
name: 'CeloDance',
description:
'Send、Vote and Earn rewards within one wallet',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you've used the wrong character here for the , after 'Send'.
Also please change the casing to 'Send, vote, and earn'

icon: CELODANCE,
canConnect: () => true,
showInList: () => true,
listPriority: () => 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please set this to 1

props
) => (
<svg width="981px" height="981px" viewBox="0 0 981 981" fill="#ffffff" style={{ height: '42px', width: '42px' }} version="1.1" xmlns="http://www.w3.org/2000/svg">

Copy link
Contributor

Choose a reason for hiding this comment

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

please clean up the spacing

@@ -481,3 +481,21 @@ export const CHROME_EXTENSION_STORE: React.FC<React.SVGProps<SVGSVGElement>> = (
/>
</svg>
);

export const CELODANCE: React.FC<React.SVGProps<SVGSVGElement>> = (
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to CELO_DANCE

@@ -356,3 +356,85 @@ export class WalletConnectConnector implements Connector {
return wallet.close();
}
}

export class CeloDanceConnector implements Connector {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have you created a new connector? It seems to be just a copy of another connector

@jmrossy
Copy link
Contributor

jmrossy commented Sep 8, 2021

@1035569003 In addition to the changes I requested, we'll need to chat with you briefly on Discord before we accept code changes. Your Github account is brand new and has no name. It simply looks too suspicious.

@bi23com
Copy link
Contributor

bi23com commented Sep 9, 2021

@1035569003 In addition to the changes I requested, we'll need to chat with you briefly on Discord before we accept code changes. Your Github account is brand new and has no name. It simply looks too suspicious.

hi,it's our new developer and he use his new account. sure please contact us in discord, the handle is Emma | Bi23 Labs#9980. thank you very much!

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.

None yet

3 participants