Remove SweetAlert

[Soare-Robert-Daniel]

Summary

• Remove SweetAlert2 dependencies
  • Replace it with a custom Popup
  • Add typescript support notations for some global variables

Will affect visual aspect of the product

YES

Screenshots

Test instructions

Test the popup when:

• Deleting a field group.
• Deleting input fields when no field is selected.

Check before Pull Request is ready:

• I have written a test and included it in this PR
• I have run all tests and they pass
• The code passes when running the PHP CodeSniffer
• Code meets WordPress Coding Standards for PHP, HTML, CSS and JS
• Security and Sanitization requirements have been followed
• I have assigned a reviewer or two to review this PR (if you're not sure who to assign, we can do this step for you)

Closes #340

[pirate-bot]

Plugin build for 98ad7e0 is ready 🛎️!

• Download PPOM - build 🔗

[carlalexander]

I don't have much to say for this PR! Looks fine. I wouldn't use chore for this commit. It's probably a refactor. I also have a few questions.

Why are naming ppomPopup using camel case when everything else uses snake case. Shouldn't we do ppom_popup?

My other question relates to the use of window?.ppomPopup? and window?.ppom_vars?. Is there are reason why we're using the optional chaining? Wouldn't we want to know if ppomPopup or ppom_vars are missing instead of ignoring it?

Finally, any way we can write some e2e test to test the change?

[abaicus]

I agree with @carlalexander here. Is there any chance that ppom_vars or the popup class instance will be undefined? IMO, the i18n.popup key should be there as long as it's used in the script. Without these being defined, the intended behavior will not be working, so instead of using optional chaining, I would rather make sure that those variables are defined.

More than that, the popup should probably be requested as a dependency of both scripts (here it is not).

---

Regarding the reasoning behind using camelCase combined with snake_case, I think that's how the script was previously localized, so it would mean changing that reference everywhere. I personally prefer using camelCase in JS usually, but I understand the difference in this specific case.

I think we should strive for one or the other in the future.

[carlalexander]

Regarding the reasoning behind using camelCase combined with snake_case, I think that's how the script was previously localized, so it would mean changing that reference everywhere. I personally prefer using camelCase in JS usually, but I understand the difference in this specific case.

I think we should strive for one or the other in the future.

Yeah, that was my main question. I just want to standardize on one type. 👍

[carlalexander]

Don't have much else to add besides my existing questions. If @abaicus feels everything is good, we're good 😃

[pirate-bot]

🎉 This PR is included in version 33.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀