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

Support sweetalert2 v8 #72

Merged
merged 6 commits into from
Mar 23, 2019
Merged

Support sweetalert2 v8 #72

merged 6 commits into from
Mar 23, 2019

Conversation

zenflow
Copy link
Member

@zenflow zenflow commented Feb 20, 2019

Closes #66

No changes to the source code were actually needed to get the tests passing; just needed to stop some removed features in the cleanSwalState test utility function.

Note that this PR keeps support for v7, but in the next major release I think we should drop it.

We should merge this with commit message feat(package): support sweetalert2 v8. The changes are backwards-compatible so we can make it a minor release.

Just one more commit to add: since the new Swal.update static method is not supported yet, I think calling it should throw an error. @limonte any thoughts?

@zenflow
Copy link
Member Author

zenflow commented Feb 20, 2019

Just one more commit to add: since the new Swal.update static method is not supported yet, I think calling it should throw an error. @limonte any thoughts?

Since supporting Swal.update will mean a huge amount of churn (almost a rewrite of the package, see the integrate-update-method branch) I would like to release this feature in the next major version, even though the changes should be backwards-compatible

edit: see issue #73

Copy link
Member

@limonte limonte left a comment

Choose a reason for hiding this comment

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

Yay! 🎉

Just one more commit to add: since the new Swal.update static method is not supported yet, I think calling it should throw an error. @limonte any thoughts?

I don't have any strong opinion here, please go ahead with whatever you think is the better way.

@limonte limonte force-pushed the support-sweetalert2-v8 branch from 71648d3 to bf52a4e Compare March 11, 2019 14:33
Copy link
Member

@limonte limonte left a comment

Choose a reason for hiding this comment

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

With jest v24 and sweetalert2@^8.3.0 the CI build is passing 🚀

@zenflow please remove WIP and merge the PR if you think it's good to go

@zenflow
Copy link
Member Author

zenflow commented Mar 22, 2019

I would like to resolve issue #82/#83 (same issue) before attempting/making this release, but unfortunately work has been demanding more of my time lately, and this will continue for at least another few weeks.

@gverni if you have time and are keen on it (which you generally seem to be for Swal2 😃), would you like to resolve issue #82/#83 and see this PR across the finish line?

Aside from that issue, I think this PR is ready to be merged and released

@limonte
Copy link
Member

limonte commented Mar 23, 2019

#82 and #83 are false alarms as I understand correctly, should be fixed by f891fbd

@limonte limonte changed the title [WIP] Support sweetalert2 v8 Support sweetalert2 v8 Mar 23, 2019
@limonte limonte merged commit a8e9356 into master Mar 23, 2019
@limonte limonte deleted the support-sweetalert2-v8 branch March 23, 2019 10:41
@zenflow
Copy link
Member Author

zenflow commented Mar 23, 2019

🎉 This PR is included in version 1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants