-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Look into upgrading to sweetalert2 #84
Comments
@daattali I can get this done with non-minified sources asap. |
I've started work on this a few weeks ago (haven't committed to git because it's not especially special code), and I did get the basics to work, that's easy enough. However, the main thing is to make sure all the old parameters work, callbacks work, and that all the UI bug fixes that were implemented over time are not going to re-appear from the migration. I do plan on finishing it at some point, but if you'd like to take the lead on the migration then I'd certainly welcome that. |
Another issue I ran into (there may be others) is chaining modals -- I just opened an issue with sweetalert2, I hope they'll fix it sweetalert2/sweetalert2#2730 In the original sweetalert this was also an issue, but someone (not in the shiny community) created the SwalService code that could handle queues. I was expecting sweetalert2 to handle queues properly, but it doesn't seem to be the case. All such regressions need to be handled before upgrading |
The author of sweetalert2 confirmed that they will not fix these issues, and because this would constitute a non trivial regression, I do consider this a blocking issue. A custom queue mechanism would need to be implemented in shinyalert, similar to how the current SwalService's queue works. |
Seems trivial enough to modify swalservice to work with sweetalert2 instead? I'm trying to do that on codepen right now. |
https://codepen.io/Rajat-Parajuli/pen/zYQroQq Was simple enough but I might be missing some edge cases. I have also commented out some shiny specific operations in between. |
Has more options and will hopefully fix the outstanding issues without introducing any new bugs
The text was updated successfully, but these errors were encountered: