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

Update manifest for standalone PWA #43

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wackerow
Copy link

Issue being addressed

This wallet is a great option to instantly onboard users with a new Ethereum account, simply by going to a website. One downside with the current approach is the risk of losing the private key stored in locale storage, either from the user clearing website data, or the data automatically being cleared from the browser after a certain amount of time. This can result in potential loss-of-funds if steps are not first taken to back up this key, or migrate to a more permanent wallet solution.

Proposed solution

Using the web manifest file we can enable a standalone progressive web app (PWA). This allows users on certain browsers/devices to "install" the web app as a standalone application.

I have been testing this branch out using an iPhone with Safari. Using a preview deploy at https://punkwallet.netlify.app/ I used the "share" button in mobile Safari and then "Add to Home Screen." I sent some Goerli ETH to the address I was presented with, and then tried a few tests:

  • Let the app sit for a couple weeks
  • Cleared all website data and cookies for Safari
  • Force closed the PWA multiple times
  • Rebuilt the site with a slightly different branch, to test if site updates would affect this

None of the above have caused any issues (iPhone + Safari), and so far the address remains the same, and my balance is still showing correctly.

PR updates

  • Updated manifest.json file to use "display": "standalone", and also added higher-resolution image assets and screenshot previews for the PWA.

Additional notes

  • I have not had the opportunity to test this on Android.
  • Testing iPhone Safari, it appears users will need to save this as a PWA before receiving funds, since the PWA appears to generate a new key in local storage at time of installation (the key from the browser instance will not be migrated).
  • Tested this on Brave with macOS, and unfortunately it appears to share the local storage data with the browser profile... which means installing the PWA will bring the same key from the browser, but will also clear if Brave site data is cleared.

If desired, we could try to add a callout to help clarify for users how this works.

seeing if change is propagated to previously installed PWA versions of the app
seeing if change is propagated to previously installed PWA versions of the app
@austintgriffith
Copy link
Contributor

maybe we should come up with a new domain/name for this and just have it be the "pwa wallet"?

Get it into the wild and use it on some phones, without overwriting how we already use the punkwallet.io

I have instantwallet.io maybe we could deploy this there?

@wackerow
Copy link
Author

Sure, down to keep it isolated from the current domain... And @austintgriffith, that quick test of just changing some text did propagate to the PWA I have installed. I'll revert it back to the usual text.

And an aside, am I missing a way to toggle light/dark mode on PunkWallet as is? If so, I'm happy to put up a separate PR that brings this back for users. I played around with it locally and deployed the initial test of this with dark mode enabled... worth noting that when I reverted those commits, the changes propagated as well, but there seems to still be a cookie stuck telling parts of the app to use the dark theme. If we had a button available to toggle this shouldn't be an issue.

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.

2 participants