-
Notifications
You must be signed in to change notification settings - Fork 38
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
Automagically enqueue popups when detected during page load. #543
Comments
@danieliser Is this a duplicate of #96 or would #96 be one more checkbox for this issue? |
Maybe close that one and move its notes here as a subtask. |
@danieliser Knowing we are removing the old form integration system, do we still need that step of checking for the thank you option? Additionally, should we check for forms on a page that one of the popups has a form submission trigger for? |
@fpcorso Good point, but removing and deprecating are 2 different things. To fully remove it we'd have to do migration routines for all of the orignal form integrations, pulling settings from each form and mapping it over all their popups (maybe). So we should likely contend that it will still be functioning for some time and would need to catch those as well. |
@danieliser Okay, that sounds good for my first question. Can you answer my second question? |
@danieliser Also, in addition to my previous question: Was reviewing the code and it seems like the PUM_Site_* classes will handle everything as long as I run So, if we have a filter for the content that scans for a PM class, we would just need to call that for the needed popup, right? Or am I missing a piece? |
@fpcorso first question, maybe. We could probably do that as an upgrade later as well. And yes |
@danieliser For within I.e., should I add a conditional such as |
@danieliser In addition to previous questions: What about enabled/disabled? Should we only preload when the popup is enabled? I can see it both ways but I think we should go with only loading if enabled. |
Ehh, the latter is more feature complete, but it becomes difficult to properly scan all the popups needed early enough that they can include their needed assets. To clarify, if a popup contains a form for example, and that form needs assets enqueued before wp_head, and we don't scan its shortcode in a popup until that popup gets enqueud in the loop somewhere it might fail to function properly.
I think this one is more simple, we gave them an explicit method to disable a popup, we should 100% respect that. So no I wouldn't scan for a disabled popup. |
The text was updated successfully, but these errors were encountered: