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: surplus modal flickering #3124

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

alfetopito
Copy link
Collaborator

Summary

Fixes medium issue #1 from #3105

Supersedes #3122

As far as I tested, it behaves ok.
It's not easy to reproduce.

To Test

  1. Place order which will generate surplus (increase price impact to 50%)
  2. Do NOT close the confirmation modal
  3. Wait for it to match
  • Modal is displayed WITHOUT flickering

@vercel
Copy link

vercel bot commented Sep 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview Sep 8, 2023 1:47pm

🌃 Cosmos ↗︎

@alfetopito alfetopito self-assigned this Sep 8, 2023
@alfetopito alfetopito mentioned this pull request Sep 8, 2023
@alfetopito alfetopito requested a review from a team September 8, 2023 14:11
@alfetopito alfetopito added the RELEASE Included in the release that is being closed label Sep 8, 2023
@alfetopito
Copy link
Collaborator Author

Optimistically merging as it's a low impact change.
Worst case it won't break the behaviour, even if it doesn't fix the issue.

Please post merge review

@alfetopito alfetopito merged commit 7d5b7ce into release/1.47.0 Sep 8, 2023
9 checks passed
@alfetopito alfetopito deleted the fix-confirmation-modal-flicker branch September 8, 2023 14:13
@github-actions github-actions bot locked and limited conversation to collaborators Sep 8, 2023
surplusAmount,
showSurplus,
}),
[surplusFiatValue, showFiatValue, surplusToken, surplusAmount, showSurplus]
Copy link
Contributor

Choose a reason for hiding this comment

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

How this is solving the issue?

I mean, this is just memorizing the result.
if it was true the theory that this is because showSurplus is true, then false, then true again (because of the USD), how is meoizing helping?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤷

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RELEASE Included in the release that is being closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants