Rather than rolling our own custom browser, we should be using Views by default. This allows us to re-use things like AJAX paging, filtering, etc without having to do it ourselves.

There are several very similar inter-connected issues related to the browser and/or Views integration but I wanted this to be a new issue to discuss.

The tricky part of this is that we need a reliable way for the user to be able to 'select' the file from the view. If the View is using field output, we can provide a 'Select in media browser' field that should be a part of the default view. If the view is using entity-based file output, then we can add a 'Output this file to select in media browser' option to the File view row plugin. We could also provide a new 'Media browser' view row plugin that emulates the output currently used in the custom media browser.

CommentFileSizeAuthor
#33 css_margin-1224766-33.patch618 bytesidflood
#33 css_margin-1224766-33-before.jpg82.57 KBidflood
#33 css_margin-1224766-33-after.jpg79.45 KBidflood
#32 Media_browser_default_view_broken_missing_handler.png153.14 KBfinn lewis
#26 1224766-26-replace_media_browser_library_with_views.patch14.71 KBrobeano
#24 1224766-24-replace_media_browser_library_with_views.patch14.65 KBrobeano
#21 1224766_update.diff5.25 KBarthurf
#20 1224766_update.diff9.43 KBarthurf
#19 1224766-19-replace_media_browser_library_with_views.patch11.93 KBrobeano
#18 1224766-18-replace_media_browser_library_with_views.patch8.12 KBbecw
#4 1224766-media-browser-replace-with-views.patch4.65 KBdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Component: Code » Media Browser
keesee’s picture

keesee commented

+1. I agree, Dave. It would also allow easier customizations for admins who would like to tweak the browser or allow them to use the multitude of views plugins to change the display.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs work
StatusFileSize
new 1224766-media-browser-replace-with-views.patch4.65 KB

Here's the progress I've made 'emulating' the media browser using views. Still needs some work.

Only local images are allowed.

entrigan’s picture

entrigan commented

@#4, did you possibly forget to add some files before rolling that patch? I cannot find views_plugin_style_media_browse.inc

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Yep I did forget it - that's why I had the foresight to leave it as needs work. :)

finn lewis’s picture

finn lewis
Location Oxford
commented

Sounds like this is where it's all happening! I'm looking forward to testing a views based media browser.

aaron’s picture

aaron commented

looking great! can't wait to review the real patch...

hernani’s picture

hernani commented

Sub

effulgentsia’s picture

effulgentsia commented

Great to see some proof of concept here. Before investing too much time in working with the current, crufty, media browser code though, I think we should work on finishing #1139514: Overhaul the media browser code to not use an iframe, and be more understandable, maintainable, and extendable. If we do that right, then swapping in a view for the library tab shouldn't be too painful (though there's always some pain getting all the AJAX to play well together).

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

This won't depend on #1139514: Overhaul the media browser code to not use an iframe, and be more understandable, maintainable, and extendable at all and won't touch media browser code at all.

jean.cedric.t’s picture

jean.cedric.t commented

@Dave :
You still work on this, i guess, and i need this, the ability to filter media by tags thanks to "Views", and naturally, in no time…
Can I Help you in some manners ? (I am dev)

csevb10’s picture

csevb10 commented

We needed this exact functionality for another project, so we built it and - for the time being - released it as a separate module. We'd love to help in any way we can to help get this integrated with Media. We're at your disposal for this, and we'll push to get anything done in a timely fashion.

Check out what we have now: http://drupal.org/project/views_media_browser

Let us know what we can do to help with this and if what we have is a reasonable start. If you don't like the direction we've gone - or you can't find a way to have us help you get it into shape for use in Media - we'll leave ours up so people can leverage it in the interim.
Thanks!
--Bill

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue tags: +Media Sprint 2011

.

muriqui’s picture

muriqui commented

subscribe... this will be awesome.

becw’s picture

becw commented
Assigned: Unassigned » becw

I'm reviewing and updating this patch.

becw’s picture

becw commented

Here's a spiffed up version of Dave Reid's patch from #4. I've added a Views display plugin; any view that uses this display plugin will show up as a tab in the media browser. I've also neatened up the style plugin, added comments, and put the views plugins in the media namespace.

This patch is incomplete; it doesn't actually display the contents of 'media browser' views in the media browser. #1289872: Use ctools plugins to provide add-ons to the media browser would require moving or rewriting the display code, so I will just do that first.

becw’s picture

becw commented
StatusFileSize
new 1224766-18-replace_media_browser_library_with_views.patch8.12 KB

Gah, I didn't attach the file.

robeano’s picture

robeano commented
Assigned: becw » robeano
Status: Needs work » Needs review
StatusFileSize
new 1224766-19-replace_media_browser_library_with_views.patch11.93 KB

I reviewed becw's patch and added some additional changes.

* added a dependency for Views module
* added a default view for the media browser
* support for additional views when exported and added to media/views/
* made a minor fix so views sees the media plugins

Now, the media_default view does display the thumbnails but it does not function like the original Library tab just yet. Additional issues will be created to manage those changes.

arthurf’s picture

arthurf commented
StatusFileSize
new 1224766_update.diff9.43 KB

Abstracted the view names and cleaned up somethings. Needs to be implemented with the files in more appropriate directories.

arthurf’s picture

arthurf commented
StatusFileSize
new 1224766_update.diff5.25 KB

Rolling again

robeano’s picture

robeano commented
Status: Needs review » Reviewed & tested by the community

Tested successfully!

Woohoo! We've got a Views support and a default view which can replace the original Library tab in the Media Browser.

There is more work to do so that this default view allows for selecting items, paging through view within the browser. Those will be handled in separate issues.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/includes/media.browser.incundefined
@@ -270,6 +290,17 @@ function media_media_browser_plugin_view($plugin_name, $params) {
     break;
+    default:
+      // Is this coming from our implemetnation of views?
+      if (! empty($params['#views_view_name'])) {
+        if (($view = views_get_view($params['#views_view_name']))) {
+          return array(
+            '#title' => t('View'),
+            '#markup' => $view->preview($params['#views_view_display_id']), // . '<div class="form-actions form-wrapper"></div>',
+          );
+        }
+      }
+    break;

The 'break' statements needs to be indented two spaces.

+++ b/media.infoundefined
@@ -4,8 +4,14 @@ package = Media
+files[] = media.views.inc

This file doesn't contain any classes - I don't think it needs to be listed here?

The patch also seems to be missing the default views?

robeano’s picture

robeano commented
Status: Needs work » Needs review
StatusFileSize
new 1224766-24-replace_media_browser_library_with_views.patch14.65 KB

Fixed changes suggested in #1224766-23: Remove default media browser and replace with a default view including adding the missing default view.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Needs work 
+++ b/includes/media.browser.incundefined
@@ -284,6 +304,17 @@ function media_media_browser_plugin_view($plugin_name, $params) {
+      // Is this coming from our implemetnation of views?
+      if (! empty($params['#views_view_name'])) {

Extra space and misspelled 'implementation'

+++ b/media.infoundefined
@@ -4,8 +4,13 @@ package = Media
+files[] = media_views_plugin_display_media_browser.inc
+files[] = media_views_plugin_style_media_browser.inc

Should these files be in includes/ or plugins/ since they're auto-loadable.

+++ b/media.infoundefined
@@ -4,8 +4,13 @@ package = Media
+files[] = includes/media.theme.inc

I should have pointed out this file is also redundant in .info file.

robeano’s picture

robeano commented
Status: Needs work » Needs review
StatusFileSize
new 1224766-26-replace_media_browser_library_with_views.patch14.71 KB

Changes from comment #25 included. I'd like one more review. This is a big 'un.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Overall I don't see anything horribly wrong anymore - could use some manual testing?

finn lewis’s picture

finn lewis
Location Oxford
commented

Very excited to see progress on this, so managed to do a quick test before bed.

Patch in #26 applied fine to the git checkout of 7.x2.x branch.

Used 'drush site-install' to set up a fresh site.

Added an image field with media browser widget to the article content type.

Default view shows up on media browser tab ok - but can't seem to select the images, and there is no submit button.

Attempting to expose some filters on the default view does not appear work just yet - the filters are exposed ok - but submitting the exposed filter form renders the site home page in the overlay iframe.

Reading back over the issue, I'm not sure if any of what I tested is meant to work yet - just thought I'd do a quick test and see how far I could get, so I hope this helps and look forward to testing it again in the morrow.

Thanks for all the work so far on this!

mstrelan’s picture

mstrelan commented

Same results as #28.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Needs work

Had a good chance to review this and found some issues, fixed them myself, and then committed the patch:
1. Fixed the location of the 'cancel' button on the View.
2. Fixed the extra closing div tag in the view template
3. Fixed a PHP strict warning due to the get_row_class() method not using the same parameters as its parent class views_plugin_style.
4. Actually added some filters to the view on File name CONTAINS and File type selection.
5. Enabled AJAX on the view so that it works for paging and auto-submits the filters.

I committed the following patch to 7.x-2.x after extensive testing. AWESOME WORK EVERYONE!
http://drupalcode.org/project/media.git/commit/5663f4d

As follow ups:
1. Fix the way media items are 'selected' to be more like the current browser library.
2. Remove the custom library code and completely replace it with the view.
3. Allow View titles to alter its own tab title: #1292932: Unable to override media browser tab titles from render/view
4. We probably need to address some file organization in the module. We have stuff in includes/ and views/.
5. The media browser needs to filter files that are not using hidden stream wrappers.
6. The media browser needs to filter based on file type or file extensions like it currently does.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

SCREENSHOT!

Only local images are allowed.

finn lewis’s picture

finn lewis
Location Oxford
commented
StatusFileSize
new Media_browser_default_view_broken_missing_handler.png153.14 KB

This is amazing! Fantastic work media sprint people!

I now have the view, with customised exposed filters, and selecting an image completes the expected action of dropping said image into the image field from which the media browser was launched.

Two issues I noticed were:
- When entering text into and exposed filter field, for example the file name, each time you stop typing, the filter is applied to the view (which is nice), but the focus of that form field is lost, so you need to click back into the text field to continue typing.
- The default view appears to have a Broken/missing handler on both the Filter Criteria and Sort Criteria.

Screenshot attached.

I'm sure these are both trivial enough and am looking forward to being able to use the media browser to filter hundreds or thousands of images and files to make reusing images and files a reality in D7.
Thanks again people! :)

idflood’s picture

idflood commented
StatusFileSize
new css_margin-1224766-33-after.jpg79.45 KB
new css_margin-1224766-33-before.jpg82.57 KB
new css_margin-1224766-33.patch618 bytes

nice : )
I have some style issue on firefox 6. Here is a patch that changes some margin and also apply a clearfix method to the "float container".

edit: would it be possible to modify a little the view so it can become easily like in http://drupal.org/files/issues/03_grid_view.jpg ?
#865006: Designs for library and wysiwyg

robeano’s picture

robeano commented
Status: Needs work » Fixed

Thanks for everyone's hard work on this issue! Please post new issues instead of posting bugs to this issue moving forward.

All outstanding tasks, bugs and features have been moved into their own new issues:

For comment #30:

#1293670: Views Library Needs to Allow Filter by File Type and Extension
#1293668: Views Library Needs to Hide Temporary Files
#1293666: General Code Review and Cleanup for 2.x
#1293658: Fix Item Select in Views Library
#1293660: Remove old media browser library plugin

For comment #32:

#1293676: Views Library Filter Bugs

For comment #33:

#1293680: Views Libary in Firefox 6
#1293692: Update Views Library to Match UX Design Grid View

ZuluWarrior’s picture

ZuluWarrior commented

Sub

arthurf’s picture

arthurf commented

This is blocked by: http://drupal.org/node/1293658 We need to make sure that the submit/cancel buttons are functional before we can replace the stock library view.

Status: Fixed » Closed (fixed)

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

jp.stacey’s picture

jp.stacey commented

For reference, the ticket mentioned by @arthurf in comment #36, which I worried might still be a blocker, is fixed: #1293658: Fix Item Select in Views Library - presumably the tickets @robeano mentions in #34 should get crossed off by automatic formatting as they too get fixed.

ZuluWarrior’s picture

ZuluWarrior commented

So, forgive me for being dense, is this issue resolved now? With the patched mentioned above can I get a views based media file selector?

Does it honour the file upload directory (if supplied) of the media field?

arthurf’s picture

arthurf commented

Yes- fix is in the dev branch

jenlampton’s picture

jenlampton
she / her
commented

Looks like the non-views Library never got removed. Is that part of this issue, or will that be done in #1293660: Remove old media browser library plugin?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Correct, we still plan to remove the old tab with #1293660: Remove old media browser library plugin which is blocked by #1293908: Respect file type restrictions in media browser library view.

gmclelland’s picture

gmclelland commented

To address what @finn.lewis said in #32

When entering text into and exposed filter field, for example the file name, each time you stop typing, the filter is applied to the view (which is nice), but the focus of that form field is lost, so you need to click back into the text field to continue typing.

Might explain why this is happening http://drupal.org/node/1319528#comment-5605672