Skip to content

Conversation

@lissavxo
Copy link
Collaborator

Related to #

Description

Fix widget not firing success

Test plan

Test widget and button make sure onSuccess works

@Klakurka
Copy link
Member

What's the context for this?

@lissavxo
Copy link
Collaborator Author

What's the context for this?

Widget is not detecting success

@Klakurka
Copy link
Member

What's the context for this?

Widget is not detecting success

Let's create tasks for issues. Any idea what version this bug was introduced in?

@lissavxo
Copy link
Collaborator Author

What's the context for this?

Widget is not detecting success

Let's create tasks for issues. Any idea what version this bug was introduced in?
I think in this commit
4cf73ef#diff-6dd4c4ad8059858d3ddf9f0b751afa81a93a9950678ce2b4610ff6768a704568

@lissavxo lissavxo force-pushed the fix/widget-on-success branch from 5ef3fcd to e792d7a Compare July 23, 2025 19:07
@Klakurka Klakurka requested review from chedieck and Copilot July 23, 2025 19:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the WidgetContainer component where the newTxs context was not properly handling cases when external transaction state management wasn't provided, causing the onSuccess callback to fail.

  • Introduces internal state management for newTxs when external state isn't provided
  • Updates all references to use the appropriate transaction state (internal or external)
  • Ensures consistent behavior for transaction handling regardless of how the component is used
Comments suppressed due to low confidence (2)

react/lib/components/Widget/WidgetContainer.tsx:135

  • [nitpick] The variable name thisNewTxs is unclear and inconsistent with the naming pattern used for currencyObj (which uses just the base name). Consider renaming to newTxs or currentNewTxs for better clarity.
    const thisNewTxs = setNewTxs ? newTxs : internalNewTxs;

react/lib/components/Widget/WidgetContainer.tsx:136

  • [nitpick] The variable name thisSetNewTxs is unclear and inconsistent with the naming pattern used for setCurrencyObj. Consider renaming to setNewTxs or setCurrentNewTxs for better clarity and consistency.
    const thisSetNewTxs = setNewTxs ?? setInternalNewTxs;

@chedieck chedieck merged commit 1fa2495 into master Jul 29, 2025
2 checks passed
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