search

stackernews / stacker.news

Internet communities that pay you Bitcoin
https://stacker.news
MIT License
403 stars 105 forks source link

Use refs for modals #1200

Closed ekzyis closed 4 weeks ago

ekzyis commented 1 month ago

Description

This fixes concurrent showModal calls with stale values which lead to modals getting replaced instead of stacked.

fixes https://github.com/stackernews/stacker.news/pull/1194#issuecomment-2135756183

Additional Context

Checklist

Are your changes backwards compatible? Please answer below:

Yes

Did you QA this? Could we deploy this straight to production? Please answer below:

Searched for showModal calls and thus tested modals for QR codes, custom zaps, zoomable images, avatar upload, delete, anon info, "you will lose your work"-modal. There are more but looks good to me

For frontend changes: Tested on mobile? Please answer below:

Did you introduce any new environment variables? If so, call them out explicitly here:

huumn commented 1 month ago

Curious. How does this fix the issue described in the comment? That issue was unrelated to modals afaict

ekzyis commented 1 month ago

Curious. How does this fix the issue described in the comment? That issue was unrelated to modals afaict

It's related to modals because the payments weren't lost but their errors opened up a modal but you never saw all of them because only the modal for the most recent error was shown. The previous modals were simply replaced without running the onClose of the previous ones. So it only looked like they were lost, but they were still pending.[^1]

[^1]: But they should get marked as failed with "payment was interrupted" as the reason