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(web-wallet): disable button while loading #4052

Merged
merged 4 commits into from
Mar 20, 2025
Merged

feat(web-wallet): disable button while loading #4052

merged 4 commits into from
Mar 20, 2025

Conversation

arein
Copy link
Contributor

@arein arein commented Mar 20, 2025

Description

Web wallets can only be opened once a connection to WalletConnect has been established. Currently the UX is confusing as the button can be clicked whilst loading with no effect. This change disables the button until it is actually clickable.

Type of change

  • Chore (non-breaking change that addresses non-functional tasks, maintenance, or code quality improvements)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Associated Issues

For Linear issues: Closes APKT-xxx
For GH issues: closes #...

Showcase (Optional)

web-loading.mp4

Checklist

  • Code in this PR is covered by automated tests (Unit tests, E2E tests)
  • My changes generate no new warnings
  • I have reviewed my own code
  • I have filled out all required sections
  • I have tested my changes on the preview link
  • Approver of this PR confirms that the changes are tested on the preview link

Copy link

changeset-bot bot commented Mar 20, 2025

🦋 Changeset detected

Latest commit: 67da85c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@reown/appkit-scaffold-ui Patch
@reown/appkit-adapter-ethers Patch
@reown/appkit-adapter-ethers5 Patch
@reown/appkit-adapter-wagmi Patch
@reown/appkit Patch
@reown/appkit-cdn Patch
@reown/appkit-adapter-bitcoin Patch
@reown/appkit-adapter-solana Patch
@reown/appkit-experimental Patch

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

Copy link

vercel bot commented Mar 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
appkit-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 20, 2025 2:22am
appkit-laboratory ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 20, 2025 2:22am
12 Skipped Deployments
Name Status Preview Comments Updated (UTC)
appkit-basic-ep ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-basic-ep-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-basic-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-basic-sign-client-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-basic-up-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-ethers5-bera ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-nansen-demo ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-vue-solana ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
appkit-wagmi-cdn-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
ethereum-provider-wagmi-example ⬜️ Ignored (Inspect) Visit Preview Mar 20, 2025 2:22am
next-wagmi-solana-bitcoin-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am
vue-wagmi-example ⬜️ Ignored (Inspect) Mar 20, 2025 2:22am

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the web wallet UX by disabling the "Open" button while the wallet connection is being established. Key changes include adding tests to verify button states, introducing an isLoading state in the wallet component, and integrating this state into the widget’s button disabled logic.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/scaffold-ui/test/partials/w3m-connecting-wc-web.test.ts Added tests to verify that the "Open" button is enabled when a URI is available and disabled otherwise.
packages/scaffold-ui/src/partials/w3m-connecting-wc-web/index.ts Introduced the isLoading state and update mechanism for controlling the button's clickable state.
packages/scaffold-ui/src/utils/w3m-connecting-widget/index.ts Updated the button's disabled attribute logic to consider the new isLoading state.

@arein arein enabled auto-merge (squash) March 20, 2025 01:57
Copy link
Contributor

github-actions bot commented Mar 20, 2025

Warnings
⚠️ File packages/scaffold-ui/src/partials/w3m-connecting-wc-web/index.ts contains a KEY or SECRET
⚠️ File packages/scaffold-ui/src/utils/w3m-connecting-widget/index.ts contains a KEY or SECRET

Generated by 🚫 dangerJS against 67da85c

Copy link
Contributor

github-actions bot commented Mar 20, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 74.82% 24449 / 32675
🔵 Statements 74.82% 24449 / 32675
🔵 Functions 66% 2058 / 3118
🔵 Branches 82.18% 4581 / 5574
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/scaffold-ui/src/partials/w3m-connecting-wc-web/index.ts 88.37% 66.66% 75% 88.37% 16-17, 27, 54-55
packages/scaffold-ui/src/utils/w3m-connecting-widget/index.ts 80.15% 75.75% 66.66% 80.15% 81-85, 94-95, 97-98, 107-109, 123-124, 190, 209-210, 226-234
Generated in workflow #10359 for commit 67da85c by the Vitest Coverage Report Action

@@ -0,0 +1,5 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to select all packages while creating changeset 🙏🏽

'@reown/appkit-scaffold-ui': patch
---

disable `open` button until connection established
Copy link
Contributor

Choose a reason for hiding this comment

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

We generally follow a practice where explanations start with capital letters unlike the commits for changeset summary

@arein arein merged commit e7480ec into main Mar 20, 2025
31 checks passed
@arein arein deleted the feat/loading branch March 20, 2025 07:04
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