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

feat: use new UX flow for connecting wallet #619

Merged
merged 51 commits into from
Sep 26, 2024
Merged

Conversation

sidvishnoi
Copy link
Member

@sidvishnoi sidvishnoi commented Sep 20, 2024

Context

Changes proposed in this pull request

  • Hide public key part from initial setup screen
  • Update connectWallet to support automatic key addition to wallet
    • automatic key addition not implemented in this PR, so it'll throw
    • but then it'll show the key for manual copy-paste like before (that's how the failure to add key automatically is designed, just that it won't fail in future)
    • Next connectWallet attempts won't try to add the key, until popup is closed.
  • Update copy, and move strings to _locales/en/messages.json.
  • Add more validations
  • Disable submit button in there's errors or missing details but with following difference from linked issue:
    • It'll initially not disable the submit button (no input), but if user tries to submit, then it'll show errors inline and then disable the button until correct data is provided. This provides a better user experience, telling why user can't submit, instead of blanket button disable. Disables button on start as @RabebOthmani asked.
  • Rewrite ConnectWalletForm: Remove react-hook-form from ConnectWalletForm component, as it was causing too many re-renders (and validation re-runs - like fetching wallet) when input changed (buggy library). It was re-rendering the entire ConnectWalletForm component, not just inputs.

To reviewers:

Demo: #619 (comment)

  • Bibi: Please check the demo video and copy in src/_locales/en/messages.json
  • Timia, Arpi: Everything else 😄, but main files:
    • src/background/services/openPayments.ts
    • src/popup/components/ConnectWalletForm.tsx

@github-actions github-actions bot added the area: popup Improvements or additions to extension popup label Sep 20, 2024
Copy link

github-actions bot commented Sep 20, 2024

Extension builds preview

Name Link
Latest commit 0d4e508
Latest job logs Run #11049219336
BadgeDownload
BadgeDownload

@github-actions github-actions bot added the area: background Improvements or additions to extension background script label Sep 20, 2024
@sidvishnoi

This comment was marked as outdated.

fix: continue connect if key add succeeds

code simplification, cleanup
@sidvishnoi sidvishnoi force-pushed the setup-page/new-flow branch 2 times, most recently from 4d4b3b3 to c29a7f7 Compare September 23, 2024 13:09
remove unnecessary div
more cleanup (try to reduce diff size)
undo some of above cleanup attempts (no benefit of diff size)
@RabebOthmani
Copy link
Collaborator

@sidvishnoi can we update the copy please and the layout (amount + renew) as provided on 565 . Thanks!

src/_locales/en/messages.json Outdated Show resolved Hide resolved
src/_locales/en/messages.json Outdated Show resolved Hide resolved
@sidvishnoi
Copy link
Member Author

sidvishnoi commented Sep 25, 2024

Demo (final)

(ignore 00:40 to 00:50 part; i had key already added, so it didn't' follow the path this PR adds support for - it went to connect consent directly)

Screencast.from.25-09-24.05.45.59.PM.IST.webm

Copy link
Member Author

@sidvishnoi sidvishnoi left a comment

Choose a reason for hiding this comment

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

test-e2e

@sidvishnoi sidvishnoi merged commit 999fa36 into main Sep 26, 2024
9 checks passed
@sidvishnoi sidvishnoi deleted the setup-page/new-flow branch September 26, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: background Improvements or additions to extension background script area: i18n area: popup Improvements or additions to extension popup area: tests Improvements or additions to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Setup]: Remove public key from setup UI initial view [UX Improvement] Setup page
3 participants