Skip to content

Conversation

@gregfromstl
Copy link
Contributor

@gregfromstl gregfromstl commented Dec 5, 2024

PR-Codex overview

This PR focuses on enhancing the user experience during the sign-in process by propagating error messages to the UI and improving the handling of wallet connection states.

Detailed summary

  • Updated is-ecosystem-wallet.ts to accept Wallet or string as a parameter.
  • Modified SignatureScreen.tsx to manage error states and display error messages.
  • Added data-testid attributes for testing purposes.
  • Enhanced tests in SignatureScreen.test.tsx to cover various scenarios including error handling and wallet disconnection.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@linear
Copy link

linear bot commented Dec 5, 2024

@gregfromstl gregfromstl self-assigned this Dec 5, 2024
@changeset-bot
Copy link

changeset-bot bot commented Dec 5, 2024

🦋 Changeset detected

Latest commit: 279cb6f

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

This PR includes changesets to release 1 package
Name Type
thirdweb 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

@vercel
Copy link

vercel bot commented Dec 5, 2024

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

Name Status Preview Comments Updated (UTC)
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 5:31am
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 5:31am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 5:31am
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 5:31am

@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 5, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 47.96 KB (0%) 960 ms (0%) 1.9 s (+11.28% 🔺) 2.8 s
thirdweb (cjs) 107.42 KB (0%) 2.2 s (0%) 3.1 s (-10.68% 🔽) 5.2 s
thirdweb (minimal + tree-shaking) 5.58 KB (0%) 112 ms (0%) 70 ms (-40.09% 🔽) 181 ms
thirdweb/chains (tree-shaking) 506 B (0%) 10 ms (0%) 123 ms (+385.77% 🔺) 133 ms
thirdweb/react (minimal + tree-shaking) 18.3 KB (0%) 367 ms (0%) 315 ms (+63.67% 🔺) 681 ms

@codecov
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 45.89%. Comparing base (91f3b4b) to head (279cb6f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...t/web/ui/ConnectWallet/screens/SignatureScreen.tsx 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5621      +/-   ##
==========================================
+ Coverage   44.92%   45.89%   +0.96%     
==========================================
  Files        1083     1083              
  Lines       56326    56333       +7     
  Branches     4061     4124      +63     
==========================================
+ Hits        25306    25853     +547     
+ Misses      30337    29788     -549     
- Partials      683      692       +9     
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from 91f3b4b
packages 41.15% <83.33%> (+1.19%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...rdweb/src/wallets/ecosystem/is-ecosystem-wallet.ts 100.00% <ø> (ø)
...t/web/ui/ConnectWallet/screens/SignatureScreen.tsx 91.69% <83.33%> (+89.66%) ⬆️

... and 16 files with indirect coverage changes

Copy link
Member

@joaquim-verges joaquim-verges left a comment

Choose a reason for hiding this comment

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

ty!!

Copy link
Contributor Author

gregfromstl commented Dec 5, 2024

Merge activity

  • Dec 5, 12:01 AM EST: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 5, 12:12 AM EST: A user added this pull request to the Graphite merge queue.
  • Dec 5, 12:32 AM EST: A user merged this pull request with the Graphite merge queue.

<!-- start pr-codex -->

## PR-Codex overview
This PR focuses on enhancing the user interface for error handling during the sign-in process in the `SignatureScreen` component of the `thirdweb` wallet. It propagates error messages to the UI, improving user feedback when sign-in fails.

### Detailed summary
- Updated `walletId` parameter to accept `Wallet` or `string` in `is-ecosystem-wallet.ts`.
- Changed state management in `SignatureScreen.tsx` to include an `error` state.
- Modified sign-in logic to set error messages on failure.
- Enhanced UI to display error messages and loading state.
- Added `data-testid` attributes for testing.
- Updated tests in `SignatureScreen.test.tsx` to cover new error handling scenarios and UI changes.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Adds the pull request to Graphite's merge queue. packages SDK Involves changes to the thirdweb SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants