Skip to content

[MNY-232] SDK: Fix ExecutingTxScreen missing back button in TransactionWidget #8193

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.

Sign up for GitHub

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
Oct 6, 2025
Merged

[MNY-232] SDK: Fix ExecutingTxScreen missing back button in TransactionWidget #8193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions packages/thirdweb/src/react/web/ui/Bridge/TransactionWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,9 @@
if (screen.id === "execute-tx") {
return (
<ExecutingTxScreen
onBack={() => {
setScreen({ id: "init-ui" });
}}

Check warning on line 775 in packages/thirdweb/src/react/web/ui/Bridge/TransactionWidget.tsx

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/react/web/ui/Bridge/TransactionWidget.tsx#L773-L775 

Added lines #L773 - L775 were not covered by tests
Comment on lines +773 to +775
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 6, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

LGTM: back action to init-ui; add coverage

Passing onBack to reset to "init-ui" is correct for recovery. Please add a test asserting the back button sets the screen back from "execute-tx".

Quick grep to help find all call sites to cover:

🏁 Script executed:

#!/bin/bash
rg -nP -C2 '<ExecutingTxScreen\b'

Length of output: 997

Add tests for back action in ExecutingTxScreen

Add tests verifying that clicking the back button in <ExecutingTxScreen>
– in TransactionModal it resets the screen to "deposit",
– in TransactionWidget it resets the screen to { id: "init-ui" }.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 773-775: packages/thirdweb/src/react/web/ui/Bridge/TransactionWidget.tsx#L773-L775
Added lines #L773 - L775 were not covered by tests

🤖 Prompt for AI Agents 
In packages/thirdweb/src/react/web/ui/Bridge/TransactionWidget.tsx around lines
773-775, the review requests tests for the back action: add unit tests that
render ExecutingTxScreen inside both TransactionModal and TransactionWidget,
simulate clicking the back button, and assert the resulting screen state;
specifically, in TransactionModal the screen should reset to the string
"deposit", and in TransactionWidget the screen should reset to an object { id:
"init-ui" }. Implement tests that (1) mount or render each parent component with
a controlled screen state or mock state setter, (2) find and click the back
button in ExecutingTxScreen, and (3) assert the screen setter was called with
the expected value ("deposit" for TransactionModal, { id: "init-ui" } for
TransactionWidget), using your test utilities/mocks consistent with the repo
testing patterns.

closeModal={() => {
setScreen({ id: "init-ui" });
}}
Expand Down
2 changes: 1 addition & 1 deletion packages/thirdweb/src/react/web/ui/TransactionButton/ExecutingScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function ExecutingTxScreen(props: {
tx: PreparedTransaction;
closeModal: () => void;
onTxSent: (data: WaitForReceiptOptions) => void;
onBack?: () => void;
onBack: (() => void) | undefined;
windowAdapter: WindowAdapter;
}) {
const sendTxCore = useSendTransaction({
Expand Down
3 changes: 3 additions & 0 deletions packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@
if (screen === "execute-tx") {
return (
<ExecutingTxScreen
onBack={() => {
setScreen("deposit");
}}

Check warning on line 138 in packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx#L136-L138 

Added lines #L136 - L138 were not covered by tests
Comment on lines +136 to +138
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 6, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

LGTM: back action wired for deposit flow; add a test

Wiring onBack to return to "deposit" looks correct. Please add a simple state-transition test to cover this path.

Example check:

🏁 Script executed:

#!/bin/bash
# List usages to guide adding tests for back navigation in deposit modal
rg -nP -C2 '<ExecutingTxScreen\b' packages/thirdweb/src/react/web/ui

Length of output: 997

Add state-transition test for back action
Add a unit test in packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.test.tsx that mounts TransactionModal, navigates to "execute-tx", invokes the onBack callback on ExecutingTxScreen, and asserts that screen resets to "deposit".

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 136-138: packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx#L136-L138
Added lines #L136 - L138 were not covered by tests

closeModal={props.onClose}
onTxSent={props.onTxSent}
tx={props.tx}
Expand Down
Loading