-
Notifications
You must be signed in to change notification settings - Fork 141
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
refactor: onboarding after install #1307
Conversation
🦋 Changeset detectedLatest commit: ce7dfa0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/blockstack/stacks-wallet-web/GHHfpfydpfbKy2ZTLBe4VFjJwSek |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work @fbwoolf—I've added some comments wrt readability and keeping with the patterns being used currently.
@@ -6,7 +6,7 @@ const manifest = { | |||
author: 'Hiro PBC', | |||
description: | |||
'Stacks Wallet. Use the Stacks blockchain to access privacy-friendly apps, and keep data in your control.', | |||
permissions: ['activeTab'], | |||
permissions: ['tabs'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to change it to this to query all tabs. I think activeTab
only allows you to have access to the active tab.
@kyranjamie one thing I noticed a little weird in testing this with the test-app is that when back at that app page the wallet is loaded but I still have to select an account before it moves past the sign up page. It seems like it should always just default to select the first indexed account to move an app past auth? |
I think for first time registrations this makes a lot of sense |
523db7c
to
33ad682
Compare
33ad682
to
14c15f0
Compare
Been working on a solution for this, do either of you know why logging |
14c15f0
to
5031f95
Compare
5031f95
to
097234e
Compare
097234e
to
5e3f83f
Compare
5e3f83f
to
56a7fbe
Compare
56a7fbe
to
565eb25
Compare
565eb25
to
677b225
Compare
677b225
to
ce7dfa0
Compare
a2b1ab4
to
0b65737
Compare
Going to close this PR, it is really outdated. Will be discussed further here: #1808 |
This PR changes the onboarding flow so that now after install the user is directed back to their app tab and it reloads the page so the extension can be detected as installed.
Issues:
hirosystems/connect#144
hirosystems/connect#98
Testing:
Sign up
againI think ultimately the goal is to extract out the concept of the
Sign up
button in the test-app and make it available via the connect library (@jasperjansz new designs for onboarding), but this is a good first step.cc/ @aulneau @kyranjamie @fbwoolf