Skip to content

[Refactor] Remove unused ConnectUIContext and refactor wallet connection components #3449

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

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

jnsdls
Copy link
Member

@jnsdls jnsdls commented Jun 21, 2024

This pull request aims to clean up the codebase by removing unused packages and files. The following tasks were carried out:

What changed?

  1. Deleted the useWalletConnectionCtx.ts file from the thirdweb package.
  2. Deleted the wallet-connection.tsx file from the thirdweb package.
  3. Updated ConnectButton.tsx, AllWalletsUI.tsx, AnyWalletConnectUI.tsx, ConnectEmbed.tsx, and other UI components to remove references to the deleted files.
  4. Modified various other files in the thirdweb and react packages to ensure that no part of the codebase relies on the removed files.

How to test?

  1. Run the project and navigate through the UI to ensure that the wallet connection features are functioning as expected without any errors.
  2. Check the console for any warnings or errors related to missing files or dependencies.

Why make this change?

Removing unused packages and files helps to streamline the codebase, improving maintainability, and potentially reducing build times. This cleanup is part of ongoing efforts to ensure the codebase remains clean and efficient.



PR-Codex overview

This PR refactors context usage in the connect wallet UI, updates in-app wallet locale handling, and optimizes code structure.

Detailed summary

  • Refactored context usage in connect wallet UI
  • Updated in-app wallet locale handling
  • Optimized code structure

The following files were skipped due to too many changes: packages/thirdweb/src/react/web/wallets/shared/GetStartedScreen.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/InjectedConnectUI.tsx, packages/thirdweb/src/react/web/wallets/shared/CoinbaseSDKConnection.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/WalletEntryButton.tsx, packages/thirdweb/src/react/web/wallets/in-app/InAppWalletSelectionUI.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx, packages/thirdweb/src/react/web/wallets/ecosystem/EcosystemWalletConnectUI.tsx, packages/thirdweb/src/react/web/wallets/shared/SocialLogin.tsx, packages/thirdweb/src/react/web/wallets/in-app/InAppWalletConnectUI.tsx, packages/thirdweb/src/react/web/wallets/ecosystem/EcosystemWalletFormUI.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/StartScreen.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModal.tsx, packages/thirdweb/src/react/web/wallets/shared/ConnectWalletSocialOptions.tsx, packages/thirdweb/src/react/web/wallets/shared/OTPLoginUI.tsx, packages/thirdweb/src/react/web/wallets/in-app/InAppWalletFormUI.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/SmartWalletConnectUI.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalInline.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AnyWalletConnectUI.tsx, packages/thirdweb/src/react/web/wallets/shared/WalletConnectConnection.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalContent.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/WalletSelector.tsx

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

Copy link

vercel bot commented Jun 21, 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 23, 2024 0:09am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 23, 2024 0:09am

Copy link

changeset-bot bot commented Jun 21, 2024

🦋 Changeset detected

Latest commit: eaa5093

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

Copy link
Contributor

graphite-app bot commented Jun 21, 2024

Your org requires 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.

@jnsdls jnsdls marked this pull request as ready for review June 21, 2024 23:05
@github-actions github-actions bot added the TS SDK Involves changes to the v5 TypeScript SDK. label Jun 21, 2024
@jnsdls jnsdls requested a review from MananTank as a code owner June 21, 2024 23:05
Copy link
Member Author

jnsdls commented Jun 21, 2024

Copy link

codspeed-hq bot commented Jun 21, 2024

CodSpeed Performance Report

Merging #3449 will not alter performance

Comparing refactor/remove-unused-context (eaa5093) with main (e743489)

Summary

✅ 9 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jun 21, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 40.63 KB (0%) 813 ms (0%) 911 ms (-2.7% 🔽) 1.8 s
thirdweb (cjs) 91.84 KB (0%) 1.9 s (0%) 1.9 s (+12.26% 🔺) 3.7 s
thirdweb (minimal + tree-shaking) 4.79 KB (0%) 96 ms (0%) 32 ms (-15.62% 🔽) 128 ms
thirdweb/chains (tree-shaking) 423 B (0%) 10 ms (0%) 26 ms (+54.55% 🔺) 36 ms
thirdweb/react (minimal + tree-shaking) 13.49 KB (0%) 270 ms (0%) 131 ms (+38.53% 🔺) 401 ms

Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 35.49433% with 398 lines in your changes missing coverage. Please review.

Project coverage is 61.01%. Comparing base (e743489) to head (eaa5093).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3449      +/-   ##
==========================================
- Coverage   61.14%   61.01%   -0.14%     
==========================================
  Files         891      889       -2     
  Lines       67375    67693     +318     
  Branches     3498     3498              
==========================================
+ Hits        41196    41301     +105     
- Misses      25496    25709     +213     
  Partials      683      683              
Flag Coverage Δ *Carryforward flag
legacy_packages 65.60% <ø> (ø) Carriedforward from e743489
packages 60.13% <35.49%> (-0.16%) ⬇️

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

Files Coverage Δ
.../react/web/ui/ConnectWallet/Modal/ConnectModal.tsx 85.33% <100.00%> (+5.89%) ⬆️
...b/src/react/web/ui/ConnectWallet/ConnectButton.tsx 57.70% <95.45%> (-0.82%) ⬇️
...t/web/ui/ConnectWallet/Modal/DeepLinkConnectUI.tsx 36.47% <33.33%> (+0.83%) ⬆️
...rdweb/src/react/web/ui/ConnectWallet/Modal/TOS.tsx 16.94% <33.33%> (ø)
...eb/src/react/web/ui/ConnectWallet/Modal/screen.tsx 78.04% <83.33%> (+0.83%) ⬆️
...t/web/ui/ConnectWallet/screens/SignatureScreen.tsx 15.80% <0.00%> (-0.26%) ⬇️
...eact/web/wallets/shared/locale/getConnectLocale.ts 30.00% <33.33%> (ø)
.../src/react/web/wallets/shared/GetStartedScreen.tsx 23.40% <57.14%> (+0.55%) ⬆️
.../src/react/web/wallets/shared/ConnectingScreen.tsx 28.69% <60.00%> (+3.24%) ⬆️
...c/react/web/wallets/in-app/useInAppWalletLocale.ts 39.13% <40.00%> (-5.87%) ⬇️
... and 14 more

... and 2 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.

very nice, more in line with my react native impl too.

one thing i want to do in both react native and react is export every single one of our 'screens' and useful components we use in there. This will help a lot.

ex. SendFunds - ReceiveFunds - ViewFunds should all be exported, embeddable components. So we need to make sure the inputs to these components make sense to be used standalone

Copy link
Contributor

graphite-app bot commented Jun 23, 2024

Merge activity

…ion components (#3449)

This pull request aims to clean up the codebase by removing unused packages and files. The following tasks were carried out:

### What changed?

1. Deleted the `useWalletConnectionCtx.ts` file from the `thirdweb` package.
2. Deleted the `wallet-connection.tsx` file from the `thirdweb` package.
3. Updated `ConnectButton.tsx`, `AllWalletsUI.tsx`, `AnyWalletConnectUI.tsx`, `ConnectEmbed.tsx`, and other UI components to remove references to the deleted files.
4. Modified various other files in the `thirdweb` and `react` packages to ensure that no part of the codebase relies on the removed files.

### How to test?

1. Run the project and navigate through the UI to ensure that the wallet connection features are functioning as expected without any errors.
2. Check the console for any warnings or errors related to missing files or dependencies.

### Why make this change?

Removing unused packages and files helps to streamline the codebase, improving maintainability, and potentially reducing build times. This cleanup is part of ongoing efforts to ensure the codebase remains clean and efficient.

---

<!-- start pr-codex -->

---

## PR-Codex overview
This PR refactors context usage in the connect wallet UI and updates locale types for in-app wallets.

### Detailed summary
- Refactored context usage in connect wallet UI
- Updated locale types for in-app wallets
- Renamed functions for better clarity

> The following files were skipped due to too many changes: `packages/thirdweb/src/react/web/wallets/shared/GetStartedScreen.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/InjectedConnectUI.tsx`, `packages/thirdweb/src/react/web/wallets/shared/CoinbaseSDKConnection.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/WalletEntryButton.tsx`, `packages/thirdweb/src/react/web/wallets/in-app/InAppWalletSelectionUI.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx`, `packages/thirdweb/src/react/web/wallets/ecosystem/EcosystemWalletConnectUI.tsx`, `packages/thirdweb/src/react/web/wallets/shared/SocialLogin.tsx`, `packages/thirdweb/src/react/web/wallets/in-app/InAppWalletConnectUI.tsx`, `packages/thirdweb/src/react/web/wallets/ecosystem/EcosystemWalletFormUI.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/screens/StartScreen.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModal.tsx`, `packages/thirdweb/src/react/web/wallets/shared/ConnectWalletSocialOptions.tsx`, `packages/thirdweb/src/react/web/wallets/shared/OTPLoginUI.tsx`, `packages/thirdweb/src/react/web/wallets/in-app/InAppWalletFormUI.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/SmartWalletConnectUI.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalInline.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AnyWalletConnectUI.tsx`, `packages/thirdweb/src/react/web/wallets/shared/WalletConnectConnection.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalContent.tsx`, `packages/thirdweb/src/react/web/ui/ConnectWallet/WalletSelector.tsx`

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

<!-- end pr-codex -->
@jnsdls jnsdls force-pushed the refactor/remove-unused-context branch from c6ce8c3 to eaa5093 Compare June 23, 2024 00:05
@graphite-app graphite-app bot merged commit eaa5093 into main Jun 23, 2024
30 of 31 checks passed
@graphite-app graphite-app bot deleted the refactor/remove-unused-context branch June 23, 2024 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TS SDK Involves changes to the v5 TypeScript SDK.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants