-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Breaking change: close() as instance method #1379
Conversation
Pull Request Test Coverage Report for Build 4516
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good!
-
closeModal
andcloseToast
were not deprecated and removing then would be an unexpected breaking change. Even though they were not documented, some people might use them. Let's not break anything without strong reasons for that. -
Please add the test proving that the desired functionality from Calling swal.close() does not call the onClose callback #919 works as expected
resolve the promise with {} (no value or dismiss)
:-) Congratulations!!!
…On Sat, 12 Jan 2019, 19:40 Limon Monte, ***@***.***> wrote:
[image: image]
<https://user-images.githubusercontent.com/6059356/51077696-afbe8800-16b2-11e9-97ea-6b798e45eff1.png>
Congrats, everyone!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1379 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQTDdLHkI0tKEi1DUwHlPGvvz7qBWpCFks5vCjpKgaJpZM4Z7_nT>
.
|
Hi @limonte sorry it took me so long, but I was trying to find the most correct way to achieve:
What I did, is to add Alternatively, I tried also to save references to To be honest, I'm not really happy with that, but I couldn't think of any better way of achieving that goal. Feel free to let me know if there is one. |
The failure on CI is obviously due to the fact that i'm always resolving the promise at the end of |
I really didn't like to use I also moved the logic for handling |
Given #1383 I can simplify even more the prototype of the |
That's a possible change, yeah. But let's agree on it before doing any changes on this PR. |
Created a new object to hold privateMethods (similar to existing privateProps) and added to this object the methods for swalPromise
* Added two optional arguments to close() to handle resolve value and reject value * Added privateMethods object to call resolve / reject for swal promise
6a668c5
to
bad3ab5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work, @gverni!
Do I understand correctly that this PR doesn't contain any breaking changes?
If so, please remove "Breaking change" form the PR title.
thanks @limonte!
The breaking change is in the fact that now if you call |
BREAKING CHANGE: close() as instance method (#1379)
BREAKING CHANGE: close() as instance method (#1379)
# [8.0.0](v7.33.1...v8.0.0) (2019-01-19) * BREAKING CHANGE: remove getButtonsWrapper() ([c93b5e3](c93b5e3)) * BREAKING CHANGE: close() as instance method (#1379) ([2519c17](2519c17)), closes [#1379](#1379) * BREAKING CHANGE: replace deprecated `jsnext:main` with `module` in package.json (#1378) ([1785905](1785905)), closes [#1378](#1378) * BREAKING CHANGE: drop Bower support (#1377) ([cb4ef28](cb4ef28)), closes [#1377](#1377) * BREAKING CHANGE: remove withNoNewKeyword enhancer (#1372) ([f581352](f581352)), closes [#1372](#1372) * BREAKING CHANGE: remove swal.noop() ([40d6fbb](40d6fbb)) * BREAKING CHANGE: rename $swal2-validationerror -> $swal2-validation-message (#1370) ([9d1b13b](9d1b13b)), closes [#1370](#1370) * BREAKING CHANGE: inputValidator and preConfirm should always resolve (#1383) ([fc70cf9](fc70cf9)), closes [#1383](#1383) * BREAKING CHANGE: remove setDefault and resetDefaults (#1365) ([97c1d7c](97c1d7c)), closes [#1365](#1365) * BREAKING CHANGE: remove extraParams (#1363) ([5125491](5125491)), closes [#1363](#1363) * BREAKING CHANGE: remove showValidationError and resetValidationError (#1367) ([50a1eff](50a1eff)), closes [#1367](#1367) * BREAKING CHANGE: remove useRejections and expectRejections (#1362) ([f050caf](f050caf)), closes [#1362](#1362) * BREAKING CHANGE: dismissReason: overlay -> backdrop (#1360) ([d05bf33](d05bf33)), closes [#1360](#1360) * BREAKING CHANGE: drop Android 4.4 support (#1359) ([c0eddf3](c0eddf3)), closes [#1359](#1359) ### Features * **api:** add update() method ([#1186](#1186)) ([348e8b7](348e8b7)) ### BREAKING CHANGES * close() as instance method (#1379) * drop Android 4.4 support (#1359) * replace deprecated `jsnext:main` with `module` in package.json (#1378) * drop Bower support (#1377) * remove withNoNewKeyword enhancer (#1372) * remove swal.noop() * rename $swal2-validationerror -> $swal2-validation-message (#1370) * inputValidator and preConfirm should always resolve (#1383) * remove setDefault and resetDefaults (#1365) * remove extraParams (#1363) * remove showValidationError and resetValidationError (#1367) * remove useRejections and expectRejections (#1362) * dismissReason: overlay -> backdrop (#1360) * remove getButtonsWrapper()
🎉 This has been shipped in version 8.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #919
I noticed the TODO comment on:
sweetalert2/src/instanceMethods/_main.js
Line 46 in f76808e
and I took the opportunity of the fix for issue #919 to refactor
close()
as instance method.I also removed the aliases
closeModal()
andcloseToast()
that were not used / documented anywhere.@limonte @zenflow let me know if I went too far and i'm happy to implement to implement the fix for
close()
as static method.