Skip to content

Conversation

@MananTank
Copy link
Member

@MananTank MananTank commented Jun 10, 2024

PR-Codex overview

This PR removes the feature that automatically sets another connected wallet as active when disconnecting the current active wallet. It also stops saving the personal wallet separately in the connected wallets list.

Detailed summary

  • Removed feature to set another connected wallet as active upon disconnecting current active wallet
  • Personal wallet no longer saved separately in connected wallets list

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

@vercel
Copy link

vercel bot commented Jun 10, 2024

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

Name Status Preview Comments Updated (UTC)
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 11, 2024 4:52pm

@changeset-bot
Copy link

changeset-bot bot commented Jun 10, 2024

🦋 Changeset detected

Latest commit: 39d98da

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

This PR includes changesets to release 8 packages
Name Type
thirdweb Patch
@thirdweb-dev/sdk Patch
@thirdweb-dev/cli Patch
@thirdweb-dev/react-core Patch
@thirdweb-dev/react Patch
@thirdweb-dev/unity-js-bridge Patch
@thirdweb-dev/wallets Patch
@thirdweb-dev/auth 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

@codspeed-hq
Copy link

codspeed-hq bot commented Jun 10, 2024

CodSpeed Performance Report

Merging #3264 will not alter performance

Comparing mnn/fix-smart-disconnect (39d98da) with main (ce45a79)

Summary

✅ 9 untouched benchmarks

@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 40.33 KB (0%) 807 ms (0%) 2.4 s (+9.59% 🔺) 3.2 s
thirdweb (cjs) 90.06 KB (0%) 1.9 s (0%) 7.5 s (+29.92% 🔺) 9.3 s
thirdweb (minimal + tree-shaking) 4.75 KB (0%) 95 ms (0%) 388 ms (+208.15% 🔺) 483 ms
thirdweb/chains (tree-shaking) 423 B (0%) 10 ms (0%) 167 ms (+322.87% 🔺) 177 ms
thirdweb/react (minimal + tree-shaking) 15.75 KB (+0.09% 🔺) 315 ms (+0.09% 🔺) 496 ms (-48.98% 🔽) 811 ms

handleSetActiveWallet(activeWallet);

// add personal wallet to connected wallets list
addConnectedWallet(personalWallet);
Copy link
Member

Choose a reason for hiding this comment

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

I think we should still add the EOA as a connected wallet. What's the reasoning for removing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

reverted this change

@MananTank
Copy link
Member Author

/release-pr

@MananTank MananTank marked this pull request as ready for review June 10, 2024 22:08
@codecov
Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.22%. Comparing base (a4d041b) to head (39d98da).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3264      +/-   ##
==========================================
- Coverage   63.28%   63.22%   -0.07%     
==========================================
  Files         835      834       -1     
  Lines       63507    63398     -109     
  Branches     3433     3432       -1     
==========================================
- Hits        40191    40081     -110     
- Misses      22641    22642       +1     
  Partials      675      675              
Flag Coverage Δ *Carryforward flag
legacy_packages 65.61% <ø> (ø) Carriedforward from a4d041b
packages 62.72% <0.00%> (-0.08%) ⬇️

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

Files Coverage Δ
packages/thirdweb/src/wallets/manager/index.ts 46.15% <0.00%> (+1.01%) ⬆️

... and 4 files with indirect coverage changes

@MananTank MananTank added this pull request to the merge queue Jun 11, 2024
Merged via the queue into main with commit 3948f43 Jun 11, 2024
@MananTank MananTank deleted the mnn/fix-smart-disconnect branch June 11, 2024 16:59
@jnsdls jnsdls mentioned this pull request Jun 11, 2024
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.

4 participants