Hi,

It would be very lovely to see the Views Library plugin tabs support multi-select, so #951004: Allow selecting of multiple media items for a multi value media field in the same dialog and similar could progress without having to patch media first.

I've attached a patch that enables multi-select (test it via the Browser test-bed, as I don't know any other built-in things that sets the multiselect-param). In addition to enabling multi-select, I've taken the liberty to restructure the Javascript slightly.

It now has a 'setup' and 'select' that triggers when the page is loaded, and when the tab containing the view is selected. To enable this, I've had to build a JS settings array that contains the views names and display ids that are active.

Slightly related, I've changed the events bound to the launcher and remove-button to use event.preventDefault instead of return false, to enable Media plugins to also listen to these events (return false is equivalent to event.preventDefault(); event.stopImmediatePropagation(), and what is needed is only to prevent the default behavior of clicking the link.
I've taken the liberty to include those changes in this issue, as I see no harm in the change, and because the reason to support multiselect is to enable modules to provide better field-widgets. Widgets which could also benefit from being able to know when either the launcher or the remove-button is pressed.

Kind Regards
Morten

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Devin Carlson’s picture

anon’s picture

Status: Needs review » Needs work

This doesnt work for me.

It stills just inserts the first file.

fangel’s picture

Status: Needs work » Needs review

anon: Are you using the normal Media Browser widget? Then of course it doesnt' work - those always only load the single-select browser.

If you want to test, you'll have to use the Browser Testbed (as basically no widgets that allows Multiselect exists). Navigate to 'media/browser/testbed', type in {multiselect: true} in the 'Options (JSON)' text-area and press the link underneath labeled 'Launch Media Browser'. Then you should be able to select multiple, and when you press OK in the browser dialog, it should give you a JSON array back in the results textarea.

Alternativly you can use my sandbox multiselect widget: http://drupal.org/sandbox/fangel/1652676 to try and test it out - but if that fails it could either be this patch, or the new widget - so please test in the testbed if you want to report bugs.

Regards
Morten

anon’s picture

Fangel: Ok, so the "feature" isn't completely done yet?

I mean your patch work the same way in the normal browser, you'll get that JSON structure, but later on, it will only take the first child from the JSON and insert.

Don't see why this should be needs review, if this is implemented as it is, its still broken as you can't use it unless someone else patch the remaining parts.

fangel’s picture

anon: Well, currently the Views based browser-tabs is unable to select more than one (the deprecated library tab supports it). So this patch is a necessary first step before a generic multi-select widget can be created!

See my sandbox if you want a widget that does multi-select. But Media needs this patch before the browser is able to select more than one item at a time!

robertom’s picture

media-views-tab-multiselect.patch queued for re-testing.

robertom’s picture

If you want to test, you'll have to use the Browser Testbed (as basically no widgets that allows Multiselect exists). Navigate to 'media/browser/testbed', type in {multiselect: true} in the 'Options (JSON)' text-area and press the link underneath labeled 'Launch Media Browser'. Then you should be able to select multiple, and when you press OK in the browser dialog, it should give you a JSON array back in the results textarea.

With Browser Testbed all works as aspected...

The patch apply with some hunks, so I have recreated this (without change)

I don't have enough experience with Drupal.behaviors, so I can't review the code on this patch... I left this as "needs review", but I hope that will marked as RTBC asap ;)

Exalted’s picture

media-views-tab-multiselect.patch queued for re-testing.

Jackinloadup’s picture

drzraf’s picture

Using latest media 2.x (patch still applies perfectly):

On the UI side it appears to work well and, at the field level, it works correctly if the file field is initially empty.

But there's a bug about the way the file field is filled when you add/remove multiple files multiple times (probably an array index problem or a trouble when duplicate filenames/fid happen to be inserted)

Please note that I tested this using the fangel/1652676 media_multiselect sandbox so I'm not sure if it's related to the javascript from the patch or from the media_multiselect module itself.

  • You can reproduce by the following:
  • add 3 files
  • remove 1 file
  • add this file again

=> the file is not added again but the empty select media row persist.

fangel’s picture

That's likely related to how the JS in media_multiselect-sandbox handles stuff - do you mind creating an issue on the sandbox, then I'll take a look at it when I get the time?

ParisLiakos’s picture

i tested in the testbed and on the fields using developers tool to set multiselect param to true adding &multiselect=true to the iframe url and everything works.

i am pretty sure that #10 is not related to this patch, but i wont rtbc this, i will leave it for someone with better javascript skills to check the code..functionality is there

effulgentsia’s picture

Title: Support multiselect in Views Library plugins » Regression: Views Library doesn't support mutliselect, 1.x library did
Category: feature » bug
Priority: Normal » Critical

Thanks for the patch. Raising this to a critical bug, because it's a regression of a fundamental feature. Would be great to have some proficient JS developers review the code, and for people to report what browsers this has been verified to work successfully on. #7 contains info on how to test.

#9 works great for me on Firefox 9 on Mac.

ParisLiakos’s picture

Would be awesome to have this in next unstable version
#9: media-views_tab_multiselect-1658452-9.patch queued for re-testing.

Andrew Edwards’s picture

#9 works well for me

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

It would be great to attract more JS experts to the Media project to help review issues like this, and maybe even become co-maintainers of the project. In the meantime, I think it's better to move this through the process than wait indefinitely for a JS review, so upping to RTBC. I would not do this for a stable branch, but since 2.x is in its unstable phase anyway, I'm taking some leeway.

ParisLiakos’s picture

Status: Reviewed & tested by the community » Fixed

couldnt agree more:)
Well i made some coding standard fixes (eg indentation and spacing) and pushed it

Thanks fangel:)

Edit: commit link here
http://drupalcode.org/project/media.git/commit/9058a4c

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

I have wrapped #951004 on [] for turn #951004 into link