Skip to content

[SDK] Add onClose callback to details modal #5605

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
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/metal-mails-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
"thirdweb": patch
---

- Add onClose callback to Connect Details modal

```tsx
<ConnectButton
detailsModal={{
onClose: (screen: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than burying this in the detailsModal prop, could we make it top level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only applicable for the Modal and not the whole button, so it contextually makes sense to be nested under "detailsModal".

However it took a while for me to raise the coverage to 80% and I'm willing to do what you said to merge this PR lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm @gregfromstl actually it's cleaner if we keep it inside the detailsModal prop. Also by putting onClose at the root level it crashes with the onClose of the react native's ConnectButton. (packages/thirdweb/src/react/native/ui/connect/ConnectButton.tsx L114 )

So I'd say we keep things as is. Would you be able to compromise?

// The last screen name that was being shown when user closed the modal
console.log({ screen });
}
}}
/>
```

- Small fix for ChainIcon: Always resolve IPFS URI

- Improve test coverage
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,12 @@ export type ConnectButton_detailsModalOptions = {
*/
allowLinkingProfiles?: boolean;
};

/**
* @param screen The screen's name that was last shown when user closed the modal
* @returns
*/
onClose?: (screen: string) => void;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,17 @@ const TW_CONNECT_WALLET = "tw-connect-wallet";
* />
* ```
*
* ### Callback for when the details modal is closed
* ```tsx
* <ConnectButton
* detailsModal={{
* onClose: (screen: string) => {
* console.log({ screen });
* }
* }}
* />
* ```
*
* @param props
* Props for the `ConnectButton` component
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {
ConnectedWalletDetails,
DetailsModal,
InAppWalletUserInfo,
NetworkSwitcherButton,
StyledChevronRightIcon,
SwitchNetworkButton,
detailsBtn_formatFiatBalanceForButton,
detailsBtn_formatTokenBalanceForButton,
Expand Down Expand Up @@ -584,4 +586,57 @@ describe("Details Modal", () => {
expect(closeModalMock).toHaveBeenCalled();
});
});

it("NetworkSwitcherButton should not render if no active chain", () => {
const { container } = render(
<AccountProvider address={VITALIK_WALLET} client={client}>
<NetworkSwitcherButton
setScreen={(scr) => console.log(scr)}
disableSwitchChain={false}
displayBalanceToken={undefined}
client={client}
/>
</AccountProvider>,
);

const element = container.querySelector(
".tw-internal-network-switcher-button",
);
expect(element).toBeFalsy();
});

it("NetworkSwitcherButton should render if there is an active chain", async () => {
vi.mock(
"../../../../react/core/hooks/wallets/useActiveWalletChain.js",
() => ({
useActiveWalletChain: vi.fn(),
}),
);
vi.mocked(useActiveWalletChain).mockReturnValue(base);
const { container } = render(
<AccountProvider address={VITALIK_WALLET} client={client}>
<NetworkSwitcherButton
setScreen={(scr) => console.log(scr)}
disableSwitchChain={false}
displayBalanceToken={undefined}
client={client}
/>
</AccountProvider>,
);
await waitFor(
() => {
const element = container.querySelector(
".tw-internal-network-switcher-button",
);
expect(element).toBeTruthy();
},
{ timeout: 2000 },
);
});

it("StyledChevronRightIcon should render a svg element", () => {
const { container } = render(<StyledChevronRightIcon />);
const svg = container.querySelector("svg");
expect(svg).toBeTruthy();
});
});
Loading
Loading