Ability to create folders

Sweet! This is a nice start.

Do you see how creating it as a field will make the code better? Make sure you look at EntityFieldQuery when coding up the filter part.

A couple quick nits:

+/**
+ * put files into default folders if not yet in any folder
+ */
+function media_browser_plus_enable()
+{
+	
+}

This needs to be written... I'm a little concerned about the performance of effecting every media item as we update, let's think about this some more. Perhaps we don't need to to update it, and the query code will just assume the default if it is not set? Nah... better to set it.

+
+/**
+ * 
+ * 
+ */
+function media_browser_plus_uninstall()
+{
+	field_delete_field('field_folder');
 }

If we do this here, we should probably do it for other fields created by this mod.

Yay, our first issue, and fellow contributor! Welcome, Sebastian!

Related to this, I added a meta issue: #969062: Decide on architectural approach to field management.

Perhaps you didn't follow the install instructions? Plupload requires a 3rd party library. See the INSTALL.txt or README.txt. Also, might be good to file a ticket to write something which warned people about this.

-J

Sebastian just an FYI. Ine way i got around that iframe resize bug you mentioned was to switch to a different tab in the uploader, like web, then back to upload. I haven't investigated this bug yet. I'm pretty sure there is not an issue for it yet either.

I can't wait to try out the folders.

Thanks for all the hard work everyone!

James.elliot knows something about this, he's been working on it. We try to resize the window to fit its contents, and IE lags behind a bit in firing the event handler, or something like that.

Nice, I just tested this out! I see what you mean about the window resizing.

Here's the issue for that if someone has ideas:
#917684: plupload tab of media browser too short in IE

Fixing that is dependent really on committing this since it will make it look like poo. :(

In regards to the patch itself, a couple general things:

-- Read http://drupal.org/coding-standards
In particular, you're indentation settings in your IDE are wrong.

-- For the folder list, use an indented select list like the one provided by the taxonomy module. I don't know off the top of my head where to find this, but I think it is in options_field_widget_form();
This might be a bit of a complex approach, but if you can figure it out, it will provide a more consistent interface for this field. Maybe something with
field_attach_form('media', $fake_item_with_type_set, $form, $form_state);
But just a WAG (Wild-Ass Guess).
Eventually, we'll probably want to build a custom interface since these type of select boxes are terrible for large hierarchies.

Best,
Jacob

I'm taking a fresh pass at #917684: plupload tab of media browser too short in IE so I wouldn't consider that blocking

Wow! You've been busy. Btw, I just unblocked you I think on the window height issue.

I won't be able to look at this for a few days. Could you please try to start posting screenshots / mockups? For new features which impact the UI, it makes everyone's life easier if you can send up your plans visually before you start coding. Then if there is any discussion that needs to occur, it's not frustrating because you haven't already built the damn thing :)

I understand though that sometimes, you have to just start building it to know what it is eventually going to look like. At any rate, I'll get to this as soon as I can, and if you don't hear from me by Monday, please ping me.

Thanks!
Jacob

Before I review any further, please review the coding standards doc. It is GREAT you are contributing, and it pays off big time in the long run to get your addition into the module and not maintain a fork, but you've got to follow the conventions and use coder module. I know it's annoying, I've gotten this same lecture several times, but the people who gave it to me (although often rude and petty) were right in this regard.

I noticed right away using:

foreach() 
{

Curlies go on the same line.
And stuff like:

return array(
+      '#type' => 'select',
+      '#language' => LANGUAGE_NONE,
+      '#title' => t('Media Folder'),
+      '#fiel

(indentation is set incorrectly).

Please take a moment (it won't take but 10minutes) to just check the indentation, spacing, etc and try to submit a clean patch.

NOTE: I will still review the functionality, etc anyway I'm not a stickler like some people, but I won't commit it till it's at least close.

Peter has written a nice Drupal coding standard sniff for PHP_CodeSniffer which you can use with Eclipse, it will find all the (non-compliant) lines automatically. http://drupal.org/node/897116 Have a look!

Regards,
Luke

This is really starting to shape up.

There are some minor formatting issues in media_browser_plus.admin.js that I'm not too concerned about. (double indenting)

However, the structure in media_browser_plus.admin.js is fine as jQuery but Drupal handles its jQuery integration is a specific sense. Drupal has its own behavior system that will attach and detach behaviors on page load. jQuery(document).ready() is therefore unnecessary and contrary to the best practices for jQuery in Drupal. This is discussed partway down the page here http://drupal.org/node/756722

I would also recommend to move away from anonymous closures in your javascript. They make stack traces less descriptive and create a circular reference which can lead towards memory leaks and poor performance. This is discussed in the blog post here. http://blog.iconara.net/2006/04/17/wonderful-closures-evil-closures/

I'm also noticing that you use the each() function in jQuery when it is unnecessary.

$item.parent().children('ul').each(function(index) {
  // and toggle their display
  $(this).toggleClass('hidden');
});

This could just as easily be written like so

$item.parent().children('ul').toggleClass('hidden');

The toggleClass() will apply to all of the children, there is no need to use each() to iterate through them.

I did some work on this yesterday.

- I rewrote the Javascript to match Drupal's implementation and use the power of jQuery.

- I rewrote the AJAX callback for media_browser_plus_thumbnailsJSON() to be a page callback and not a form build operation

- I altered the CSS file to match Drupal's CSS style guide.

Duh, I forgot to add the -N switch to my cvs diff.

New patch attached

Good catch on the folder always being 1. I had used that for debugging and forgot to fix it before I rolled the patch. And I didn't realize you had added a media_browser_plus_load_multiple(). That looks promising.

I should have some time on wednesday to take a closer look at the state of the patch. I think I can even make a project branch so that we can commit the files there and no longer have to worry about the zip folder while still keeping the main branch viable.

Sorry about the delay, I've been completely swamped. I'm going to cut a branch today.

I cut a stable branch without your patch and then committed everything in #30 to HEAD for development

subscribe

Great work on this guys, thanks!

The UI enhancements are great, but the only thing I can't seem to get working is creating new folders. Right now "Media Root" is the only option. Am I missing something, or is this a known issue?

subscribing

subscribing

ready to help test once dev version can be rolled, good work on such a key feature

@sebastian using your patch I can create new folder and select one during media upload but I can't browse the library, I mean it's still a big repo that doesn't consider the taxonomy "Media Folder" structure

Yup, exactly :)
But if I select an image, adds it to the basket and click on "continue with media basket selection" I simply return on the upload tab and nothing appears in the tinymce editor

You'll need update functions for this to work on existing sites I think

This is the version with working selection.
I added "select" button to bypass media basket.
Not sure if it is better than the version that Sebastian made.

Actually I find your approach quite strange. Why don't you use the same technique for grid rendering as media_browser does? Rendering grid in js from JSON'ed media entities?
I had to make additional server call to get required data.
The other issue with Library Plus is lack of filtering by allowed media type.

First, thanks you very much. This is great!
There is a few issue i noticed:
- I can't delete medias. When i select medias and click "delete" the page reload, but nothing happen. The drupal's error log doesn't report anything.
- If there are medias in the media basket, when i click on "delete", the media basket's content start downloading.
- in "list" mode, media's deletion works
- if I click on the "list" view button, when i quit media overview's page, the button is on "list" mode when i come back, but the view is in thumbnail mode (not a big issue but that could be confusing)
- overlay positioning is wrong when i want to attach media to a field or to a wysiwyg editor. It's in the top left corner instead of page's center

That's all for now. (tested on Safari and Firefox)

I confirm that the first three issues are fixed with the latest version. Thanks !
About the position of the overlay, that's not a critical issue, but it's a strange behavior anyway.
I attached a screenshot: as you can see, the box is in the left top corner of the page. Before updating, it was at the absolute center. It's annoying when the image fields is at the bottom of the page because users have to scroll to the top to see the box.
Tested on Safari 5.0.3 and Firefox 3.6.14.

Hi,

Work perfectly well for me (Safari and Firefox). Great job! Thanks again.

This is looking really good. I've created a "folders" branch in git to continue developing this.

One thing I did notice when taking a quick look was that this appears to no integrate properly with the plupload feature of the media module. There is some sort of JS error when adding files.

The problem appears to have been an outdated version of plupload on my end. I'm going to mark this as "fixed". That way we can continue to improve this in smaller more easily tested, reviewed, and committed changes.

Folders branch is broken.
Missing 'include' directory, media_browser_plus.admin.inc media_browser_plus.folders.inc, etc

Also there is a bug in _media_browser_plus_has_unsorted_media function which prevents display of the Unsorted folder.

The correct code is:

function _media_browser_plus_has_unsorted_media(){
// loading media
$entity_controller = entity_get_controller('media');
$media_entities = $entity_controller->load(FALSE, array(), 0, 100);

Can't make a patch because there is no working version and i don't understand what repo and branch is active now:(

Subscribing

sub

+1

Where is main development of this branch going on? On github or on "folders" branch here? Are there any plans for merge of this into master?