Hi,

Now, if we want to remove the video, we need to remove the string from the input.
Would be great to have remove button if field have multiple values. I made some little patch based on field collection module. Not the best one solution I think, but it works.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

guschilds’s picture

Status: Active » Needs work

Hey kedramon,

Thanks for the patch! I've always thought this form would benefit from a remove button. I've had to add them to other forms and I know how much of a pain it is. I think following the Field Collection's lead is a good idea, I just wish it didn't require 100+ lines of code :). For that reason I'm not 100% sure yet if it's worth the added complexity, but we might as well get a working patch that others can use and we can gauge interest.

I tried your patch and ran into a couple issues:

--- sites/all/modules/contrib/youtube/youtube.module	(revision b6bcf99ba8754d4180a62881b694141486114a75)
+++ sites/all/modules/contrib/youtube/youtube.module	(revision )

Because these lines list the full path to the module in your specific installation, it doesn't apply to a checkout of the 7.x-1.x branch. To avoid this in the future, it might be easiest to, once your done with your code changes, check out the repo, apply your changes to it, and then create the patch. More information about creating patches for Drupal can be found here. Those lines should end up looking something like this:

diff --git a/youtube.module b/youtube.module
index eafddeb..58033b9 100644
--- a/youtube.module
+++ b/youtube.module

Next, once I applied your patch, I wasn't able to get it to function. I installed a clean Drupal 7 with the YouTube Field module and created an unlimited cardinality YouTube field. I applied the patch and cleared cache. When I click the "Remove" button, the field item isn't removed and this error is thrown:

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /field_youtube/ajax
StatusText: Service unavailable (with message)
ResponseText: Recoverable fatal error: Argument 1 passed to drupal_array_get_nested_value() must be of the type array, null given, called in /Users/gus/www/drupal7/sites/all/modules/youtube/youtube.module on line 676 and defined in drupal_array_get_nested_value() (line 6771 of /Users/gus/www/drupal7/includes/common.inc).

I haven't spent any time trying to debug it.

And a couple of code style / standardization comments. First:

+  $items['field_youtube/ajax'] = array(
+    'title' => 'Remove item callback',
+    'page callback' => 'field_youtube_remove_js',

Lets rename the path to youtube/ajax and the function youtube_remove_js. This actually follows the Field Collection module closer by prefixing those pieces with the name of the module.

And finally:

+      '#name' =>  'youtube_'.$delta.'_remove_button',

There should be a space before and after the . in the string concatenation. More about that in the Drupal coding standards.

Thanks again!
Gus

kedramon’s picture

Hi Gus,

I worked a bit on this task and fixed my errors, but there is one issue left,
since the video code is pre populated during validation, it is always in field delta description. Will post my patch tomorrow, maybe You give me some idea how to deal with it.

Thanks!
Vova

kedramon’s picture

Status: Needs work » Needs review
FileSize
5.89 KB

Hi,

sorry for late posting, here is my patch.

guschilds’s picture

Status: Needs review » Reviewed & tested by the community

Hi kedramon,

Nice work! I didn't run into any problems when quickly testing #3 and the code changes look good.

I'm going to mark this as RTBC and leave it open so others know they can use it, but as I mentioned before, I'm not sure that I'll be committing it to the module. It's a bummer that it takes 129 lines of new code to pull this off, increasing the size of the .module file by over 20%. I just don't think the feature is worth that increase in size and complexity.

kedramon’s picture

Hi Gus,

ok, thanks for reviewing!

guschilds’s picture

Status: Reviewed & tested by the community » Closed (works as designed)

Closing after a year of inactivity and a lack of expressed interest. The patch is still here for anyone that wants this functionality, but will likely never be committed. Thanks again, Vova.