-
Notifications
You must be signed in to change notification settings - Fork 39
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
Form integration bug fixes and improvements #889
Conversation
args.popup.length && | ||
args.popup.is($popup)) || | ||
!settings.only_in_popup | ||
) { |
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.
Fixed spaces to tabs so it shows the whole file changed. The only actual code changes is this conditional.
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.
PHPStorm doesn't allow line by line staging which is ashame, but VSCode does which is nice, you can highlight the changed lines, stage them, commit, then stage the rest of the file for a second commit.
…mine if we should process the submission Since we repeat the same code for getting the popup ID and for increasing the conversion in all integrations, we should also move that here in case we need to change how that works in the future. Issue #887
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.
Like the massive deduplication of code. Looks good!
Description
Collection of commits relating to fixing the bugs in the form integration system
Related Issue
Issues: #886, #887, #706
Types of changes
args
argument instead of using the passed form.This has been tested in the following browsers
Checklist: