IMCE integration

On the whole, this is good code. I've made a few comments below.

Architecturally though, I don't think this is the right approach. @james.elliott is reworking a lot of the bad JS in the media browser code to make it easier to work with, and I think as part of that he's going to consider how to support different browsers.

We're looking at creating a views based browser for instance, although this would only affect the library tab, not every tab. Still, if we are going to support more than one option for browsers, we should make that a pluggable hook, not hardcode "use_imce" in various places. Perhaps as a start to get this in so people can use it, we could build two patches.

One would be a new module in the contrib directory "media_imce" which would contain all the code you've put in here. And the other would be patches to media which would allow the user to pick a browser to use. I think this means a variable to set the default browser site wide and a setting at the element level to choose the browser. So:

 $form['media_add_link'] = array(
  '#type' => 'media'
  ''#media_options' => array(
      'browser' => 'imce',
      'global' => array(
        'types' => array_filter($instance['widget']['settings']['allowed_types']),
        // @todo: Not implemented
        'schemes' => $instance['widget']['settings']['allowed_schemes'],
      ),
    ),
);

The default would be site wide set to a variable, but depending on where the element was added it could choose a specific browser.

Thoughts?

diff -rNu media.20110411/includes/media.admin.inc media.new/includes/media.admin.inc
--- media.20110411/includes/media.admin.inc	2011-04-11 09:00:40.000000000 +0100

Patch should start inside the media module directory.

+++ media.new/js/media.popups.js	2011-04-13 20:38:48.331718609 +0100
@@ -129,6 +134,75 @@
+  mediaIframe.css('width', '98%');

Is this going to work with different themes?

Powered by Dreditor.

this is awesome, thanks szaszg! it's definitely been a goal of the project since the beginning to have media allow for pluggable browsers. it's great to see a push in that direction, with a usable alternative!

Look again at the coding standards and try to remove tabs, white spaces, "no newline at end of file" warnings.
You should try dreditor (in combination with firefox and chrome) to checkout your patch in here.

+1

Dreditor is an chrome and firefox extension, if you install it there will be a review button next to your uploaded patch if you click it you get red warnings when you did something wrong like using trailing whitespaces.

As it is a browser extension it doesn't matter what kinda version of drupal you're using :)

@szaszg since media 7.x-2.x is going to have some significant rewrites to the browser and surrounding JS should we hold on this patch until that gets worked out?

subscribe

when installing the sandbox module i get following error

Fatal error: Class 'MediaBrowserPlugin' not found in /www/sites/all/modules/contrib/media_browser__imce/media_imce.module on line 53

Using latest release of media

Is there any update on this integration?

Ping... Been over a year now, can we assume this isn't getting in the next release?

bummer - made sense.

subscribe

Generally folks are encouraged to Follow rather than "subscribe".

Anyways, it's been a while now. I do wonder if the bots have been engaged in doing an automated review.

The last release was from 2014-Jan-08 so it's over due for a new stable release.

Closing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team