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

TypeScript definitions are incorrect. #100

Closed
torkelrogstad opened this issue Nov 6, 2019 · 5 comments · Fixed by #101
Closed

TypeScript definitions are incorrect. #100

torkelrogstad opened this issue Nov 6, 2019 · 5 comments · Fixed by #101
Labels

Comments

@torkelrogstad
Copy link

onOpen and onClose does not take the popup as their argument, this should be updated in the definitions file.

@zenflow
Copy link
Member

zenflow commented Nov 7, 2019

The definitions file for this package doesn't define those methods.. Is this an issue for sweetalert2?

@limonte limonte closed this as completed Nov 7, 2019
@torkelrogstad
Copy link
Author

After wrapping my modals in sweetalert2-react-content the onOpen and onClose functions no longer received the modals as their argument, and I had to call Swal.getPopup to get a hold of if. So what I'm trying to say, is that this package should redefine the onOpen and onClose signatures to be () => void instead of HTMLElement => void. Similar to what you do for the other changed props: https://github.com/sweetalert2/sweetalert2-react-content/blob/master/src/sweetalert2-react-content.d.ts#L36

@limonte limonte reopened this Nov 8, 2019
@zenflow
Copy link
Member

zenflow commented Nov 8, 2019

I think it would be better to make the fix so that those methods are called with the modal as their argument, and keep things in alignment with how sweetalert2 behaves without this extension (sweetalert2-react-content).

@limonte
Copy link
Member

limonte commented Nov 8, 2019

🎉 This issue has been resolved in version 2.0.2 🎉

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 a pull request may close this issue.

3 participants