Skip to content
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

fix: send modal close #5004

Merged
merged 1 commit into from
Jul 31, 2023
Merged

fix: send modal close #5004

merged 1 commit into from
Jul 31, 2023

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Jul 31, 2023

Description

This fixes the send modal currently not closing after successful Tx.

short tl;dr:

This guy isn't defined and makes us throw, meaning the next line will never be evaluated, thus we will never close the modal.

Long tl;dr:

#4971 brought a refactor of modals, where in addition to perf. improvements, we nest all modal providers inside each other.

This theoretically works, but doesn't if you try and use useModal() twice within the same component, with two different modals. The reason for that is actually a missing key in <Provider />, meaning that each provider isn't properly identified, and calling useModal() twice will try and use the context of the previous useModal(''), resulting in an empty object context for the current modal, and useModal('qrCode') returning undefined, hence us throwing in the line mentioned above.

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

Risk

  • None

Testing

  • Do a send of any asset
  • Ensure the send modal closes after send

Engineering

  • ☝🏽

Operations

  • ☝🏽

Screenshots (if applicable)

Copy link
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@gomesalexandre gomesalexandre marked this pull request as ready for review July 31, 2023 12:51
@gomesalexandre gomesalexandre requested a review from a team as a code owner July 31, 2023 12:51
@0xApotheosis 0xApotheosis self-assigned this Jul 31, 2023
Copy link
Contributor

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

The send modal is now closing as expected.

Also tested other modals to ensure no regressions there, and all looked 🛍️

@0xApotheosis 0xApotheosis merged commit e0409c6 into develop Jul 31, 2023
3 checks passed
@0xApotheosis 0xApotheosis deleted the fix_modals_closing branch July 31, 2023 20:21
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.

Send Modal doesn't update once TX is broadcast Unable to send Osmo
2 participants