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

[SubWallet] Enable Mobile version #639

Merged

Conversation

S2kael
Copy link
Contributor

@S2kael S2kael commented Dec 23, 2022

Pull Request Summary

This is the pull request I have created for the integration of our SubWallet mobile into the Astar portal. Please merge.

Change

  • Update wallet config

@impelcrypto
Copy link
Member

impelcrypto commented Dec 23, 2022

Hi @S2kael
Thank you for your contribution. Kindly please take a look at this page and submit the required links.

@nampc1
Copy link

nampc1 commented Dec 24, 2022

Hello @impelcrypto, we are preparing the necessary information for the integration here Koniverse/SubWallet-Mobile#297. I'm wondering if we can pass the PR without having to test out every case mentioned in the doc ?

To be specific, we are having trouble getting some tokens to test out every case. We see that the Portal only requests signature from the wallet to submit transaction so technically we don't need to test with every single type of token.

SubWallet Mobile works just like SubWallet extension so we are sure it doesn't bring any strange behavior to the Portal. SubWallet extension has been working well on Astar Portal for over a year so i think it would be fine. Plus, i see that the there was only 1 commit and it was just about changing some configurations.

@impelcrypto
Copy link
Member

impelcrypto commented Dec 24, 2022

To be specific, we are having trouble getting some tokens to test out every case.

Which tokens you are having trouble for testing? I can't approve this PR without testing. Also please deploy this this branch to somewhere like Vercel, so that our team can test the functions.

CC @togamamora @fiexer @Kahonnohak

@nampc1
Copy link

nampc1 commented Dec 26, 2022

Here are the resources of the testing that we've done
https://drive.google.com/file/d/1nJmfmMxd6E9RPHj2D7yFdcXXASVZ_Nt6/view
For more detail of the testing, see this issue here
Koniverse/SubWallet-Mobile#297

We have not found ways to test out these cases (mainly because we currently don't have the corresponding tokens):
Substrate wallet:

  • XCM transfers: deposit & withdrawal
    EVM wallet:
  • ERC20 token transfer
  • XCM transfer
  • Withdrawal

Do we have to try out every single case ? Since some cases are similar and it would take us a while to get tokens

cc @impelcrypto

@impelcrypto
Copy link
Member

Thank you, I'll test it out tomorrow.

  1. Can I assume that the branch is working fine on Android as well?
  2. Could you deploy this branch to somewhere like Vercel as I requested above? so that our team can test it out easily.

@nampc1
Copy link

nampc1 commented Dec 26, 2022

  1. Yes, everything is working well on Android. Our testing team tested on both Android/iOS
  2. Yes we are working on it, we will submit a Vercel deployment soon

Thank you

@S2kael
Copy link
Contributor Author

S2kael commented Dec 26, 2022

Here is the deployment link: Vercel

@impelcrypto
Copy link
Member

impelcrypto commented Dec 27, 2022

Hi @NamPhamc99

It looks like the Approve request won't be closed after I tap either Reject or Approve.

OS: Android (Oneplus A3000)
App version: 0.2.3
Transaction: Transfer ASTR, Stake on dApp staking (native)

image

image

@nampc1
Copy link

nampc1 commented Dec 28, 2022

It's working correctly on the device that we use, here are the videos of the test cases. Can you test it with another device perhaps ?
https://drive.google.com/drive/u/2/folders/12s1DGrJEjDIYgnM-d85moiWhCyRqCa8k

Device: Xiaomi Mi 8 Lite - Android 10

@impelcrypto
Copy link
Member

impelcrypto commented Dec 28, 2022

@sirius651 @Kahonnohak @bobo-k2 @hoonsubin @togamamora @gluneau @niklabh

It's working correctly on the device that we use, here are the videos of the test cases. Can you test it with another device perhaps ?

I'd like to seek your help for testing SubWallet mobile app on our portal especially for Android.

Staging URL: https://astar-portal-wheat.vercel.app/

Copy link
Member

@impelcrypto impelcrypto left a comment

Choose a reason for hiding this comment

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

I've tested on iOS. LGTM

@sirius651
Copy link
Contributor

@NamPhamc99 Hi, regarding android, is this only for extension? not native app?

@nampc1
Copy link

nampc1 commented Dec 29, 2022

Hi @sirius651, i'm not sure i understand your question. Can you be more specific ?

@impelcrypto
Copy link
Member

@sirius651 If I understand correctly, we can test it on the Android native app.
https://play.google.com/store/apps/details?id=app.subwallet.mobile

@nampc1
Copy link

nampc1 commented Dec 29, 2022

@sirius651 If I understand correctly, we can test it on the Android native app. https://play.google.com/store/apps/details?id=app.subwallet.mobile

yes you can download it from Play Store to test it out if you want

@sirius651
Copy link
Contributor

@sirius651 If I understand correctly, we can test it on the Android native app. https://play.google.com/store/apps/details?id=app.subwallet.mobile

yes you can download it from Play Store to test it out if you want

Approved. confirmed that works well on android

@nampc1
Copy link

nampc1 commented Jan 3, 2023

Hello @impelcrypto, is there any new update regarding this PR ? Do we have to wait for the other 5 reviewers to approve ?

@impelcrypto impelcrypto merged commit bcb96f5 into AstarNetwork:main Jan 3, 2023
@impelcrypto
Copy link
Member

Hi @NamPhamc99, I've merged this PR. Please take note that we are going to deploy the latest main branch to the production page (https://portal.astar.network/) next Monday.

@nampc1
Copy link

nampc1 commented Jan 3, 2023

Thank you @impelcrypto

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

4 participants