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

Feature/okxwallet #1742

Closed
wants to merge 4 commits into from
Closed

Conversation

okxwallet
Copy link
Contributor

Description

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
  • [X]sign message
  • sign typed message
  • disconnect

@vercel
Copy link

vercel bot commented May 25, 2023

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 4, 2023 6:33am

docs/package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Adamj1232
Copy link
Member

@okxwallet 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, we can close this for now and I will update you through github and our telegram channel as we gain clarity on this feature release. 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!

@okxwallet
Copy link
Contributor Author

@okxwallet 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, we can close this for now and I will update you through github and our telegram channel as we gain clarity on this feature release. 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!

got it

@Adamj1232
Copy link
Member

@okxwallet we have a PR open to add 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
I have added the OKX download URL noted in this PR but feel free to comment or add questions. Thanks!

@Adamj1232
Copy link
Member

Closing as we now have a feature to allow this through the Injected wallets package as noted in the above comment. Feel free to continue the conversation here. Thanks!

@Adamj1232 Adamj1232 closed this Jun 12, 2023
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.

3 participants