-
Notifications
You must be signed in to change notification settings - Fork 619
[BLD-286] SDK: add stable classNames to elements in Connect UI #8065
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "thirdweb": patch | ||
| --- | ||
|
|
||
| Add `tw-` class names in connect ui | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| "use client"; | ||
| import { ChevronLeftIcon } from "@radix-ui/react-icons"; | ||
| import { lazy, Suspense, useEffect, useRef, useState } from "react"; | ||
| import { useEffect, useRef, useState } from "react"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reintroduce lazy loading for InAppWalletSelectionUI (perf regression) Switching from lazy to eager import pulls the in‑app flow into the initial bundle and penalizes TTI for users who never open it. Restore code-splitting with Suspense. -import { useEffect, useRef, useState } from "react";
+import { lazy, Suspense, useEffect, useRef, useState } from "react";-import InAppWalletSelectionUI from "../../wallets/in-app/InAppWalletSelectionUI.js";
+const InAppWalletSelectionUI = lazy(
+ () => import("../../wallets/in-app/InAppWalletSelectionUI.js"),
+);
+import { LoadingScreen } from "../../wallets/shared/LoadingScreen.js";- {wallet.id === "inApp" && props.size === "compact" ? (
- <InAppWalletSelectionUI
+ {wallet.id === "inApp" && props.size === "compact" ? (
+ <Suspense fallback={<LoadingScreen />}>
+ <InAppWalletSelectionUI
chain={props.chain}
client={props.client}
connectLocale={props.connectLocale}
disabled={props.disabled}
- done={() => props.done(wallet)}
+ done={() => props.done(wallet)}
goBack={props.goBack}
recommendedWallets={props.recommendedWallets}
- select={() => props.selectWallet(wallet)}
+ select={() => props.selectWallet(wallet)}
size={props.size}
wallet={wallet as Wallet<"inApp">}
- />
+ />
+ </Suspense>
) : (Also applies to: 21-21, 622-633 🤖 Prompt for AI Agents |
||
| import type { Chain } from "../../../../chains/types.js"; | ||
| import type { ThirdwebClient } from "../../../../client/client.js"; | ||
| import type { InjectedSupportedWalletIds } from "../../../../wallets/__generated__/wallet-ids.js"; | ||
|
|
@@ -18,7 +18,7 @@ import { | |
| } from "../../../core/design-system/index.js"; | ||
| import { useSetSelectionData } from "../../providers/wallet-ui-states-provider.js"; | ||
| import { sortWallets } from "../../utils/sortWallets.js"; | ||
| import { LoadingScreen } from "../../wallets/shared/LoadingScreen.js"; | ||
| import InAppWalletSelectionUI from "../../wallets/in-app/InAppWalletSelectionUI.js"; | ||
| import { | ||
| Container, | ||
| Line, | ||
|
|
@@ -43,10 +43,6 @@ import { PoweredByThirdweb } from "./PoweredByTW.js"; | |
| import { WalletButtonEl, WalletEntryButton } from "./WalletEntryButton.js"; | ||
| import { WalletTypeRowButton } from "./WalletTypeRowButton.js"; | ||
|
|
||
| const InAppWalletSelectionUI = /* @__PURE__ */ lazy( | ||
| () => import("../../wallets/in-app/InAppWalletSelectionUI.js"), | ||
| ); | ||
|
|
||
| // const localWalletId = "local"; | ||
| const inAppWalletId: WalletId = "inApp"; | ||
|
|
||
|
|
@@ -202,7 +198,7 @@ const WalletSelectorInner: React.FC<WalletSelectorProps> = (props) => { | |
| title={props.modalHeader.title} | ||
| /> | ||
| ) : ( | ||
| <Container center="y" flex="row" gap="xxs"> | ||
| <Container center="y" flex="row" gap="xs" className="tw-header"> | ||
| {!props.meta.titleIconUrl ? null : ( | ||
| <Img | ||
| client={props.client} | ||
|
|
@@ -222,6 +218,7 @@ const WalletSelectorInner: React.FC<WalletSelectorProps> = (props) => { | |
|
|
||
| const connectAWallet = ( | ||
| <WalletTypeRowButton | ||
| className="tw-select-connect-a-wallet-button" | ||
| client={props.client} | ||
| disabled={props.meta.requireApproval && !approvedTOS} | ||
| icon={OutlineWalletIcon} | ||
|
|
@@ -234,6 +231,7 @@ const WalletSelectorInner: React.FC<WalletSelectorProps> = (props) => { | |
|
|
||
| const newToWallets = ( | ||
| <Container | ||
| className="tw-get-started-container" | ||
| flex="row" | ||
| style={{ | ||
| justifyContent: "space-between", | ||
|
|
@@ -489,6 +487,7 @@ const WalletSelectorInner: React.FC<WalletSelectorProps> = (props) => { | |
| return ( | ||
| <Container | ||
| animate="fadein" | ||
| className="tw-wallet-selection" | ||
| flex="column" | ||
| fullHeight | ||
| scrollY | ||
|
|
@@ -620,22 +619,21 @@ const WalletSelection: React.FC<{ | |
| // data-full-width={!!walletConfig.selectUI} | ||
| > | ||
| {wallet.id === "inApp" && props.size === "compact" ? ( | ||
| <Suspense fallback={<LoadingScreen height="195px" />}> | ||
| <InAppWalletSelectionUI | ||
| chain={props.chain} | ||
| client={props.client} | ||
| connectLocale={props.connectLocale} | ||
| disabled={props.disabled} | ||
| done={() => props.done(wallet)} | ||
| goBack={props.goBack} | ||
| recommendedWallets={props.recommendedWallets} | ||
| select={() => props.selectWallet(wallet)} | ||
| size={props.size} | ||
| wallet={wallet as Wallet<"inApp">} | ||
| /> | ||
| </Suspense> | ||
| <InAppWalletSelectionUI | ||
| chain={props.chain} | ||
| client={props.client} | ||
| connectLocale={props.connectLocale} | ||
| disabled={props.disabled} | ||
| done={() => props.done(wallet)} | ||
| goBack={props.goBack} | ||
| recommendedWallets={props.recommendedWallets} | ||
| select={() => props.selectWallet(wallet)} | ||
| size={props.size} | ||
| wallet={wallet as Wallet<"inApp">} | ||
| /> | ||
| ) : ( | ||
| <WalletEntryButton | ||
| className="tw-wallet-select-wallet-button" | ||
| badge={undefined} | ||
| client={props.client} | ||
| connectLocale={props.connectLocale} | ||
|
|
@@ -655,7 +653,7 @@ const WalletSelection: React.FC<{ | |
| })} | ||
|
|
||
| {props.onShowAll && props.showAllWallets !== false && ( | ||
| <ButtonContainer> | ||
| <ButtonContainer className="tw-show-all-wallets-button"> | ||
| <WalletButtonEl onClick={props.onShowAll}> | ||
| <ShowAllWalletsIcon> | ||
| <div data-dot /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Changeset bump likely incorrect (API surface changed).
This PR appears to make
Modalrequire aclassNameprop. That’s a public API change and should be a minor bump per our rules, unless we revert to a non‑breaking optional prop. Pick one:classNameoptional and default internally so this remains patch.Apply this diff for Option A:
Run to confirm whether
classNameis required and that all usages pass it:🏁 Script executed:
Length of output: 527
🏁 Script executed:
Length of output: 8874
Bump changeset to minor — Modal's className prop is now required.
Modal's props declare
className: string(required). Found usages withoutclassNamein:Choose one:
className?: stringand default internally to keep a patch.📝 Committable suggestion