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

Conflict with Multiple File Upload field #20

Open
Avakulenko opened this issue Nov 4, 2016 · 16 comments · May be fixed by #25
Open

Conflict with Multiple File Upload field #20

Avakulenko opened this issue Nov 4, 2016 · 16 comments · May be fixed by #25

Comments

@Avakulenko
Copy link

Gravity Forms: Multiple Form Instances version: 1.1.1
Wordpress version: 4.6.1

Plugin prevents "delete file" event in the Multiple File Upload field.

@rrobinpro
Copy link

I have same problem with Multiple Form Instances version 1.1.1 and Wordpress version 4.6.1

@medfreeman
Copy link
Contributor

You can solve this by adding the following snippet in your theme's functions.php file:

if ( ! function_exists( 'gform_multiple_upload_delete_file_fix' ) ) {
	function gform_multiple_upload_delete_file_fix( $strings ) {
		if ( isset( $strings['\'gf_2\''] ) ) {
			$form_id = str_replace ( 'gf_', '', $strings['\'gf_2\'']);
			$form_id = str_replace ( '\'', '', $form_id);
			$form_id = absint( $form_id );
			if ( $form_id ) {
				$strings["onclick='gformDeleteUploadedFile(2"] = "onclick='gformDeleteUploadedFile(" . $form_id;
				$strings["onkeypress='gformDeleteUploadedFile(2"] = "onkeypress='gformDeleteUploadedFile(" . $form_id;
			}
		}
		return $strings;
	}
}

add_filter( 'gform_multiple_instances_strings', 'gform_multiple_upload_delete_file_fix' );

As you can see, my example only covers form with ID 2.
This plugin's filter 'gform_multiple_instances_strings' should obviously pass the original form id and the new random form id, so these string manipulations are not needed.

I'll try to make a few pull requests when i got more time, one to extend the filter parameters, and the second to simply add the needed string for file uploads deletion in https://github.com/tyxla/Gravity-Forms-Multiple-Form-Instances/blob/master/gravityforms-multiple-form-instances.php

@rrobinpro
Copy link

Thanks medfreeman

Your code is correct when you reload the form but not live when you upload a document.

@medfreeman
Copy link
Contributor

medfreeman commented Jan 23, 2017

Update that works with multifile uploads:

REMOVED CODE

And you'll also need this javascript snippet somewhere:

REMOVED CODE

edit: i have this functionality working on this branch of my fork, i'll submit this as a pull request when the previous PR are merged, and if the employed method is the best we can come up with, since it needs using a modified copy of a gravity forms javascript file.

@tyxla
Copy link
Owner

tyxla commented Jan 23, 2017

@medfreeman thank you for this! Would you like to contribute it as a PR so we can fix that in the next version?

@medfreeman
Copy link
Contributor

Thanks.
I'll gladly submit a PR, but it seems i've spoken too fast.
This code results in the form not submitting the uploaded file at all.
I'm looking into it, and i'll correct the code when it's done.

medfreeman added a commit to medfreeman/Gravity-Forms-Multiple-Form-Instances that referenced this issue Jan 30, 2017
…ields, add javascript to unbind gravity forms setup function, add modified javascript file from gravity forms source, add corresponding tests

fixes tyxla#20
@medfreeman
Copy link
Contributor

medfreeman commented Jan 30, 2017

I'm afraid the approach i used is the best i can come up with.
This implies having a part of a gravityforms js file in the source, with little modifications.
I'm not thrilled with it, as it could need maintenance (and could create incompatibilities with specific gravityforms versions), but the way gravity is coded now doesn't allow a better approach (do not hesitate to correct me if i'm wrong !).

@medfreeman
Copy link
Contributor

I just figured that i can do this in a simpler way.
Hook on plupload 'beforeUpload' event, change multipart_params form_id to the original form id by taking the id stored in the field named 'gform_wrapper_original_id_'
That should work and only require a few lines of code, and nothing from gravity's source.
I'll close the actual PR and try to submit a new one this afternoon.

@tyxla
Copy link
Owner

tyxla commented Feb 6, 2017

@medfreeman that would be really awesome. I'm reluctant to introduce an entire file from a premium plugin in an OSS repo :) Thank you!

@medfreeman medfreeman linked a pull request Feb 6, 2017 that will close this issue
@medfreeman
Copy link
Contributor

done!

@medfreeman
Copy link
Contributor

medfreeman commented Feb 6, 2017

there's a few spacing problems, i added the fix to the pr.

@tyxla
Copy link
Owner

tyxla commented Feb 6, 2017

Thanks @medfreeman, I appreciate your contribution! I'll have a look in the next few days.

@medfreeman
Copy link
Contributor

Hey, what do you think of my PR ?
Thanks

@tyxla
Copy link
Owner

tyxla commented Apr 4, 2017

Hey @medfreeman, thanks for your PR!

To be honest, I'm not anxious to introduce external JS files into the plugin, especially if they include functions from the original plugin. That wouldn't be a problem if Gravity Forms were open source, but unfortunately that's not the case.

@medfreeman
Copy link
Contributor

Gravity Forms is indeed opensource, GPL-3.0.
And this is about a single function.
I can add the missing credits if you'll reconsider this.
Thanks

@tyxla
Copy link
Owner

tyxla commented Apr 5, 2017

@medfreeman good call, I didn't expect for them to be GPL while being a premium plugin.
But it makes sense to use the code freely in that case, but indeed, let's add credits accordingly.

Can I recommend that you make the code compliant with the WordPress Coding Standards?
Here are some links for more information:

Thanks in advance! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants