If you select "Small" for display of an image field, nothing is displayed because that formatter isn't valid:
Undefined index: #formatter i theme_styles_field_formatter() (line 11 of .../sites/all/modules/styles/themes/styles.theme.inc).
I think the problem is the that in the function media_enable in media/media.install, the view_mode_settings for image media_small isn't set to a file style. Unfortunately, the obvious fix
$view_mode_settings['image']['media_small'] = 'styles_file_small';
this doesn't fix it because there is no small default style - see the function file_styles_styles_default_styles in styles/contrib/file_styles/includes/styles/file_styles.styles.inc
These two modules should agree on having small or medium (or both) as defaults.
Comment | File | Size | Author |
---|---|---|---|
#65 | 1051090-media_file_view_mode-56.patch | 17.27 KB | ParisLiakos |
#56 | media-file-view-mode-1051090-56.patch | 17.63 KB | Devin Carlson |
#56 | interdiff.txt | 2.37 KB | Devin Carlson |
#54 | media-file-view-mode-1051090-54.patch | 17.78 KB | Jackinloadup |
#53 | media-file-view-mode-1051090-53.patch | 17.79 KB | RobW |
Comments
Comment #1
hansfn CreditAttribution: hansfn commentedRelated Styles bug report - #872518: Undefined index: #formatter in theme_styles_field_formatter().
Comment #2
JacobSingh CreditAttribution: JacobSingh commentedYes, it's true. Here's the issue though:
It's hard to agree on generic "view modes" for media. An image can be "thumbnail", "web" "print", etc right? An Audio file could be "sample", "low-res", "high-res", etc...
So Preview is used for the browser (and could be used elsewhere), Small and Large are generic and represent a derivation of the original generally. And original is of course, the original.
I wouldn't want to remove "small" from media, because I think it would be used a lot. However, it does present a UX challenge. Should we set "small" to the generic link style for now?
Comment #3
hansfn CreditAttribution: hansfn commentedI understand that it's a problem to agree on generic "view modes" for media. My reason for this bug report is just that these two modules should agree on either just small, just medium or both. If not, the UX is terrible.
Comment #4
mattcasey CreditAttribution: mattcasey commentedJacob - Speaking theoretically, if you think you can judge what people want for Large, why can't you judge what they want for Small?
I just thought "Small" was broken until I noticed I could change the style formats. IMHO, this is a 'bug,' not a feature, as it requires additional configuration after install before it can be used.
Comment #5
Taxoman CreditAttribution: Taxoman commented+1 for that
This should perhaps also be described in the readme file (or just place a short comment there with a link back to this discussion here?)
Comment #6
JacobSingh CreditAttribution: JacobSingh commentedI think you can. The reason "small" doesn't have default formatters is because core only ships with a few image styles. In a sense, this is a styles module problem to fix. We don't want the media module declaring new formatters because then we'd have to do it for several file types.
So the fix is probably to hide the view mode or... ?
Comment #7
drew reece CreditAttribution: drew reece commentedI'm seeing this too…
How do you manually add a small style to make this work? I added a file style at admin/config/media/file-styles, called it small, but it has no effect, I can't assign an image style preset to this style either. Do we need to hack the module to make 'small' work?
Is there anyway the media module can use the image styles provided by the styles module, it feels like it is duplicating the feature of setting up 'image scaling presets'.
Comment #8
effulgentsia CreditAttribution: effulgentsia commentedI think we should add media_medium, remove media_small, and write an update function to move from the latter to the former.
Image module by default comes with "thumbnail", "medium", and "large". File Styles module, by default, adds "square thumbnail", and then makes file styles for each of these four, plus a 5th one for "original". Media module defines a default view mode "preview", instead of calling it "thumbnail" or "square thumbnail", and I think #2 provides a good answer for why that is. And Media module also adds view modes for "small", "large", and "original". Why "small" here instead of "medium"? I see no reason for this, and believe it might be a historical glitch from perhaps when Image module and Styles module used the word "small" instead of "medium".
Any objections to above?
See #1026790: Allow view modes to be dynamically defined.. But regardless of that issue, I think it makes sense for us to fix our default names to map to these other modules.
Comment #9
drew reece CreditAttribution: drew reece commentedI'm not sure I can take in what is going on in
#1026790: Allow view modes to be dynamically defined but it sounds like changing to media_medium is good enough if the formatter will work.
+1
Comment #10
JacobSingh CreditAttribution: JacobSingh commentedThe reason we have "small" and "large" is that view_modes are per entity type, not bundle. So If you have an audio file, should it be "medium" and "large". Granted, small and large is also a little strange for this case, but at least it makes some sense. Medium and large is like Starbucks coffee sizes :)
Anyway, do whatever you think is clearer, but that was the original motivation; to provide view modes which would be comprehendible across different file types.
Comment #11
effulgentsia CreditAttribution: effulgentsia commentedI'm not sure "small" and "large" makes any more or less sense than "medium" and "large". Both imply 2 different sizes, where one is bigger than the other. For whatever reason, D7 core developers working on Image module chose "medium" and "large". I don't know why, and don't care enough to find out. I think in this case, we should just be consistent. Though maybe someone wants to grab the drupal.org/project/starbucks namespace and change all the names to "tall" and "venti"? :)
Comment #12
effulgentsia CreditAttribution: effulgentsia commented[EDIT: double post. ignore this one]
Comment #13
RobW CreditAttribution: RobW commentedTo expand a little on effulgentsia in #8: If the object is to work towards media-independent names for multimedia styles, I think we should define guidelines for what certain terms mean (in Media or Styles? I am thinking this is a solely Media issue now that Media module is no longer dependent on Styles). It would be my hope that a well reasoned naming scheme for Media could eventually be adopted by other modules, working towards some sort of Drupal-wide standardization of terms.
My thoughts on image/audio/video are something like:
Drifting a little off topic, but thought I'd post as long as I was thinking about it.
Comment #14
effulgentsia CreditAttribution: effulgentsia commentedThe initial bug report of a PHP notice when a media field is set to use "Small" was fixed as part of #1086958: Switch from Media Entity to File Entity.
#1026790: Allow view modes to be dynamically defined. is fixed, so with a module like Display Suite, you can create additional view modes, like "Medium" or whatever else you want, and Media module now provides a UI for the media formatter to use that view mode, and the "Image" formatter within "Manage file display" for a given file type lets you route that to any Image style you want.
You can also configure Media's "Small" view mode to make the File visible, and then choose an Image style (or any other file formatter) for it.
So, all that's left in this issue, I think, is the UI problem of a default Media module installation creating a Small view mode and making the file hidden in that view mode. Just now, I added a code comment linking an existing @todo about that to this issue.
Therefore, I'm downgrading this issue to "normal". It would be great to get more UI input on this from people. Should we remove the default Small view mode entirely, since now administrators can make custom view modes? Should we link it to the thumbnail image style? Or to the medium image style? How are Media plugin modules dealing with something other than images implementing Small, and how can we best standardize our terminology for best UX?
#13 is a good start (thanks, RobW). To it, I would add:
As can be seen from above list, "Small" is the only view mode created by Media module, but that has no meaning to Media module itself, which I think is part of why there's confusion about what role it should play.
Comment #15
drew reece CreditAttribution: drew reece commentedIf display suite is required to make the small format work why does media create it as an option in the first place?
Media creates an option that doesn't work and requires an additional module to configure. It wouldn't be so bad if display suites wasn't so complex.
Here is an outline of what is involved in making 'small' do something with display suite (mostly for my reference).
Install Display Suite.
Go to admin/structure/ds/layout
Find the type you want to manage (eg Image), click 'Manage display'
Click the 'Small' secondary tabs at the top,
Set the file format to visible, save.
Click the manage File Display tab, Select 'Small' in the secondary tabs.
Pick a file style, save.
Now the small formatter can actually do something when a user inserts something with it selected.
The module is very promising, but why do admins need to jump through hoops to make it do what it is supposed to do?
How does any of this tie in with the file styles at admin/config/media/file-styles - it doesn't so you now have one place to specify the 'small' preset (display suite) and another for every other media preset (admin/config/media/file-styles).
Comment #16
RobW CreditAttribution: RobW commented@drew, I believe you can do something with small without display suite. In the current media dev:
I haven't checked the stable releases, but they may work the same way. Hope that helps.
Comment #17
drew reece CreditAttribution: drew reece commented@RobW
You are correct, I have the dev versions, & it is how you describe, goody.
I was wrong display suite isn't required to make the small format work. I still find it a little odd that media installs a format in a 'non-working state'.
Thanks.
Comment #18
effulgentsia CreditAttribution: effulgentsia commentedI agree, which is why this issue is still open. The question is, what is a "working state" that makes sense (see earlier comments)? Or, should we remove "small" altogether? Moving this issue to 2.x, but I'm open to backporting a solution to 1.x once we have one.
Comment #19
effulgentsia CreditAttribution: effulgentsia commentedDiscussed this in the Media sprint. Here's what we came up with:
Any feedback on this plan?
Comment #20
effulgentsia CreditAttribution: effulgentsia commentedJust to clarify, once #19 is implemented, new sites will just have Preview, Teaser, and Full. Preview is what's used in administrative interfaces (e.g., in the media browser's Library tab, and the preview shown by the media widget when editing the file field of a node). Teaser and Full could be used as needed to customize pages for site visitors, similarly to how Teaser and Full are used for nodes.
Comment #21
effulgentsia CreditAttribution: effulgentsia commentedComment #22
effulgentsia CreditAttribution: effulgentsia commentedComment #23
drew reece CreditAttribution: drew reece commented#19 seems like a good way forward.
Reducing the number of styles is not a problem since more can be added, 'not broken by default' will be great :)
Site admins can be left to wrestle with the naming of additional styles that get used across media types.
Comment #24
rickvug CreditAttribution: rickvug commented#19 is very sensible. Having a minimal amount of built in view modes means less to configure and understand for beginners and less cruft to disable for advanced users.
Comment #25
aaron CreditAttribution: aaron commentedwe just need to preserve the old view modes when updating.
Comment #26
amateescu CreditAttribution: amateescu commentedHere's a patch per @effulgentsia's suggestions on IRC.
Comment #27
mstrelan CreditAttribution: mstrelan commentedSubscribe
Comment #28
idflood CreditAttribution: idflood commentedPatch looks good. I've quickly tried to update a previous installation and the deprecated view modes are kept like they should. I've noticed some typos so here is a reroll
Comment #29
RobW CreditAttribution: RobW commentedSounds great, the move to more semantically meaningful labels divorced from what the actual presentation of those settings on individual sites is a really good thing, IMO. Labels based on context, not on what they may be configured as.
As an aside, is there any change to the recommended way to add more formatters (Display Suite/Styles/Media itself)?
Comment #30
robhamm CreditAttribution: robhamm commentedSorry--I had a stupid question here, and it looks like I answered it myself. Sorry.
Comment #31
effulgentsia CreditAttribution: effulgentsia commentedThanks amateescu and idflood for the patch, and others for the feedback.
Upon more thinking about this and a conversation in IRC, I think it's best to move Preview and Teaser into File Entity: #1296268: Add Preview and Teaser view modes.
Once that's done, then media_preview can be migrated to preview, media_small can be migrated to teaser, and media_large can be migrated to full. That then only leaves media_link and media_original that need to be retained on existing sites.
Here's a patch that enhances #28 with this. I tested it a bunch, but it could use more. Once it's ready, it should be committed after #1296268: Add Preview and Teaser view modes, but as soon after as possible, since the update function here will override preview/teaser/full settings that someone might have put into file entities if they're running the branch tip of File Entity. I think that's acceptable though, since there's a different expectation of reliability of database updates from branch tip to branch tip. Release to release is what we need to worry about more.
Comment #32
effulgentsia CreditAttribution: effulgentsia commentedMy recommendation for adding administrator-defined view modes via the UI is to use Display Suite. You can also do it in code just by implementing hook_entity_info_alter() (which is what Display Suite does). All the formatters (the "Rendered File" formatter for file and image fields and the "Media" formatter for the legacy "Multimedia asset" field) now use formatter settings for connecting to the file's view mode, and they automatically pick up on any view modes added via hook_entity_info_alter().
Comment #33
RobW CreditAttribution: RobW commentedAwesome, thanks for the reply, effulgentsia.
Comment #34
virtualgirl CreditAttribution: virtualgirl commentedI will go elsewhere if this is a low priority, ...
Is it possible to add a video popup in colorbox in the media module without having to go through the colorbox-load options in the css. Thank you for your consideration Media is a pleasure!
Comment #35
bojanz CreditAttribution: bojanz commentedPlease don't hijack issues.
Comment #36
Dave ReidComment #37
pbuyle CreditAttribution: pbuyle commentedHere is a re-roll of the patch from #31. I seems to do the job, but I didn't test deeply. The best would be to have a Media 1.x install using view modes to migrate.
Comment #38
RobW CreditAttribution: RobW commentedAh, never mind, wrote too soon.
Comment #39
RobW CreditAttribution: RobW commentedWhen I apply #37, I get default, link, and original view modes, but no teaser or full.
Comment #40
RobW CreditAttribution: RobW commentedThis ended up being my fault, missed some configuration (http://drupal.org/node/1296268#comment-5955546).
Testing now and working well.
Comment #41
mrfelton CreditAttribution: mrfelton commentedReally looking forward to seeing this get in. File view modes, formatters, render styles etc has been a big source of confusion up until now. The discussion in this ticket really helps to clarify why things are as they currently are, and the changes implemented here and in its counterpart #1296268: Add Preview and Teaser view modes seem like a very good solution that should simplify the setup and bring file view modes more inline with Core's idea of view modes for Node types.
As an asside, you can also use the Entity view modes module to add additional file view modes if you don't want to use Display Suite or custom code implementations.
Comment #42
mrfelton CreditAttribution: mrfelton commentedPatch updated to apply against latest media code.
Comment #43
mrfelton CreditAttribution: mrfelton commentedWhy is media__show_deprecated_view_modes set to TRUE? Why must these depreciated view modes still be supported? The comment in media_entity_info_alter indicates that they only need to be supported from user's coming from Media 1.x, yet we are setting this to TRUE even for users upgrading from previous 2.x versions.
Comment #44
RobW CreditAttribution: RobW commented+1 for #43. 2.x is still in unstable, so this type of change shouldn't be a big deal. Maybe we could test if there are any formatters set in the deprecated view modes and only hide if they're empty.
Comment #45
RobW CreditAttribution: RobW commentedMerged changes / re-rolled against latest dev.
Comment #47
RobW CreditAttribution: RobW commentedPasses on my local install. I think test bot is having some issues.
I have the worst luck with test bot.
Comment #49
RobW CreditAttribution: RobW commented#45: file_view_mode-1051090-45.patch queued for re-testing.
Comment #51
RobW CreditAttribution: RobW commentedThe patch isn't passing D.O. tests right now because of #1774482: Modules with no tests shouldn't actually fail.
It is valid however, applies well and passes local installation tests. Don't let that pink warning scare you away.
Comment #52
RobW CreditAttribution: RobW commentedSemi off topic cross post: I think there's a problem somewhere between this issue and the related file entity issues (#1296268: Add Preview and Teaser view modes, etc). When I try to define default file formatters in a module, either none get through, or only the ones for the originalmedia_XXX
view modes do. Cross posting because re-defining the view modes is only useful if the new ones are equally functional. See http://drupal.org/node/1320308#comment-6436464 for more info.[Edit] The problem was on my end, just a little unfamiliarity with CTools.
Comment #53
RobW CreditAttribution: RobW commentedAdded the removal of
file_entity_help()
entries for "Manage file display", which should be handled by file entity now.Comment #54
Jackinloadup CreditAttribution: Jackinloadup commentedFixed 2 whitespace issues in #53. No other changes.
Comment #55
RobW CreditAttribution: RobW commentedWhitespaaaace! (shakes fist in air)
Thanks.
Comment #56
Devin Carlson CreditAttribution: Devin Carlson commentedThe Media update functions are now up to 7203 so the update function will no longer run.
Rerolled to use
media_update_7204
.Comment #57
Devin Carlson CreditAttribution: Devin Carlson commentedTested alongside #1296268: Add Preview and Teaser view modes by configuring the file display of a number of file types, adding media which is set to display using the configured displays and then applying the patch in #56.
The patch successfully deprecated the link and original view modes and migrated all of the configuration options for media_small, media_large and media_preview to the new teaser, full and preview view modes. Media which was set to display using the deprecated link and original view modes also continued to work properly.
Comment #59
Devin Carlson CreditAttribution: Devin Carlson commentedMoving back to RTBC. This will fail without #1296268: Add Preview and Teaser view modes (both should be committed at the same time).
Comment #60
madar CreditAttribution: madar commentedI applied #56 against media-7.x-2.0-unstable6 with some offset warning
I applied http://drupal.org/node/1296268#comment-6445656 too from #1296268: Add Preview and Teaser view modes.
File preview on edit form working as expected after setting up 'Manage File Display' of image file type.
Also works on media_gallery multiedit page with multiform module.
Comment #61
RobW CreditAttribution: RobW commentedI've been using this alongside #1296268: Add Preview and Teaser view modes for a few weeks now. Very RTBC.
Comment #62
RobW CreditAttribution: RobW commentedNow that #1296268: Add Preview and Teaser view modes has gone through, #56: media-file-view-mode-1051090-56.patch queued for re-testing.
Comment #64
ParisLiakos CreditAttribution: ParisLiakos commentedneeds a reroll for sure, doesnt apply just tried
Comment #65
ParisLiakos CreditAttribution: ParisLiakos commentedlets see
Comment #67
ParisLiakos CreditAttribution: ParisLiakos commentedok, seems good, commited.
thanks everyone:)
Comment #69
sheldonkreger CreditAttribution: sheldonkreger commentedAdded related issue.