Skip to content

feat: add OneKey wallet #1739

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jun 12, 2023
Merged

feat: add OneKey wallet #1739

merged 7 commits into from
Jun 12, 2023

Conversation

@ByteZhang1024
Copy link
Contributor

@ByteZhang1024 ByteZhang1024 commented May 24, 2023

Description

Add OneKey wallet support

PLEASE NOTE- Checklist must be complete prior to review.

Checklist

  • Increment the version field in package.json of the package you have made changes in following semantic versioning and using alpha release tagging
  • Check the box that allows repo maintainers to update this PR
  • Test locally to make sure this feature/fix works
  • Run yarn check-all to confirm there are not any associated errors
  • Confirm this PR passes Circle CI checks
  • Add or update relevant information in the documentation

Docs Checklist

  • Include a screenshot of any changes (see docs README on running locally)
  • Add/update the appropriate package README (if applicable)
  • Add/update the related module in the docs demo (if applicable)
  • Add/update the related package in the docs/package.json file (if applicable)

If this PR includes changes to add an injected wallet or SDK wallet module:

Please complete the following using the internal demo package.
To run this demo use the command yarn && yarn dev to get the project running at http://localhost:8080/

Tests with demo app (injected)

  • send transaction
  • switch chains
  • sign message
  • sign typed message
  • disconnect

Tests with demo app (SDK)

  • send transaction
  • switch chains
  • sign message
  • sign typed message
  • disconnect

@vercel
Copy link

vercel bot commented May 24, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
web3-onboard-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2023 8:07pm

Adamj1232
Adamj1232 reviewed May 25, 2023
View reviewed changes
@ByteZhang1024 ByteZhang1024 requested a review from Adamj1232 May 26, 2023 09:28
@Adamj1232
Copy link
Contributor

Adamj1232 commented May 26, 2023

@ByteZhang1024 We are planning to introduce a new feature that enables the inclusion of a URL as part of the injected module. The intention behind this is to obviate the necessity of a separate module for routing users without installed wallets in the case of injected wallets. This will help immensely with Blocknative's upkeep and standardize usage for injected wallets without requiring dapp devs to install separate packages for this feature.
In terms of your PR, could you kindly restrict the changes specifically to packages/injected, packages/demo, and the documentation pages? I appreciate your cooperation.

Early next week, I will provide updates concerning the method and schedule for accommodating the webpage within the injected module. Thank you!

@Adamj1232
Copy link
Contributor

Adamj1232 commented Jun 6, 2023

@ByteZhang1024 we have a PR just merged that adds a feature to easily add this ability without creating/maintaining a new module just for adding the ability to install a wallet that is currently installed. This approach will also be a lighter lift on the dapp developer as they will not need to install any new packages or keep those packages up to date for consistent behavior and only need to add the wallet label to the displayUnavailable prop array on injected module initialization. This is the PR - #1754
Please pull in the latest develop and follow the steps to add your download url to the object and injected provider. Please also remove the module created to add this ability exposed by this new injected module feature when you have a moment.
Please feel free to reach out with any questions. Thanks!

@ByteZhang1024
Copy link
Contributor Author

ByteZhang1024 commented Jun 8, 2023

@Adamj1232 Is it OK to write docs like this.

@Adamj1232
Copy link
Contributor

Adamj1232 commented Jun 8, 2023

@ByteZhang1024 as your wallet is not a separate module we can remove the separate pages and OneKey can be added to the list here - https://github.com/blocknative/web3-onboard/blob/develop/docs/src/routes/docs/%5B...4%5Dwallets/%5B...9%5Dinjected/%2Bpage.md#injected-wallets-supported-natively

@ByteZhang1024
Copy link
Contributor Author

ByteZhang1024 commented Jun 11, 2023

@Adamj1232 done

Adamj1232
Adamj1232 approved these changes Jun 12, 2023
View reviewed changes
Copy link
Contributor

@Adamj1232 Adamj1232 left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks for the collaboration!

Adamj1232
Adamj1232 suggested changes Jun 12, 2023
View reviewed changes
Copy link
Contributor

@Adamj1232 Adamj1232 left a comment

Choose a reason for hiding this comment

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

Please bump the version on this line within the injected package.json to 2.10.1-alpha.1

@Adamj1232
Copy link
Contributor

Adamj1232 commented Jun 12, 2023

Please bump the version on this line within the injected package.json to 2.10.1-alpha.1

Took care of this and committed the updated version.

@socket-security
Copy link

socket-security bot commented Jun 12, 2023

New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives1 Size Publisher
@web3-onboard/injected-wallets 🆕 2.9.0 None +0 449 kB cmeisl

Footnotes

  1. https://docs.socket.dev ↩

@Adamj1232 Adamj1232 merged commit bf2600f into thirdweb-dev:develop Jun 12, 2023
This was referenced Jun 14, 2023
Merged
Merged
Merged
leightkt added a commit that referenced this pull request Jun 27, 2023
@leightkt @Adamj1232
@ByteZhang1024 @sanyu1225 @finessevanes
Release 2.24.2 (#1793)
1b6df15 
* update Google Analytics Script (#1770)

* feat: add OneKey wallet  (#1739)

* feat: Support OneKey wallet

* fix: OneKey docs

* Update injected package.json version

* Update demo package.json

---------

Co-authored-by: Adam Carpenter <[email protected]>

* Fix: Connect view on safari mobile & Remove unnecessary validation check for autoSelect (#1773)

* working as expected

* Bump versions

* Remove testing css

* Remove testing code from demo

* feat: add blocto wallet (#1699)

* Release 2.24.1 (develop) (#1776)

* Release 2.24.1 (docs) (#1778)

* Update +page.md (#1775)

* Feature- update appMetadata (#1779)

* update versions

* Upgrade wallet connect v2 packages (#1788)

---------

Co-authored-by: Adam Carpenter <[email protected]>
Co-authored-by: ByteZhang <[email protected]>
Co-authored-by: jimmy.pan <[email protected]>
Co-authored-by: vanes <[email protected]>
leightkt added a commit that referenced this pull request Jun 27, 2023
@leightkt @Adamj1232
@ByteZhang1024 @sanyu1225 @finessevanes
Release 2.24.2 (docs) (#1792)
346bdba 
* update Google Analytics Script (#1770)

* feat: add OneKey wallet  (#1739)

* feat: Support OneKey wallet

* fix: OneKey docs

* Update injected package.json version

* Update demo package.json

---------

Co-authored-by: Adam Carpenter <[email protected]>

* Fix: Connect view on safari mobile & Remove unnecessary validation check for autoSelect (#1773)

* working as expected

* Bump versions

* Remove testing css

* Remove testing code from demo

* feat: add blocto wallet (#1699)

* Release 2.24.1 (develop) (#1776)

* Release 2.24.1 (#1777)

* Update +page.md (#1775)

* Feature- update appMetadata (#1779)

* update versions

* Upgrade wallet connect v2 packages (#1788)

---------

Co-authored-by: Adam Carpenter <[email protected]>
Co-authored-by: ByteZhang <[email protected]>
Co-authored-by: jimmy.pan <[email protected]>
Co-authored-by: vanes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Adamj1232 Adamj1232 Adamj1232 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ByteZhang1024 @Adamj1232