First of all, great job including this functionality, it solves a ton of issues for content creators and site builders!

However, not getting it to work as I expect. i have restricted the view modes in the settings to "Ingress" (teaser), "fullstendig innhold" (full content) and WYSIWYG, but when I try to add an image my options are not these, but "Standard", "Ingress", "Forhåndsvis" (preview) and WYSIWYG. See screenshots:

ui for restricting

ui for inserting

..actually now I see that I always have "Standard", "Ingress" and "WYSIWYG" available, even if I'm not restricting to them. Have I misunderstood something here? What I typically want to to is create 2-3 specific view modes, like "content full | half | quarter width" and make only these available to insert into the WYSIWYG..

some potential clues: the site is localized, and I'm using CKEditor 4.3. Media and File Entity 2 alpha 3.

CommentFileSizeAuthor
#17 2158177-media-wysiwyg-view-mode-ui.patch21.46 KBdave reid
#12 2158177-media-wysiwyg-view-mode-ui.patch18.43 KBdave reid
#10 Selection_279.png53.17 KBdave reid
#10 2158177-media-wysiwyg-view-mode-ui.patch18.85 KBdave reid
#9 2158177-media-wysiwyg-view-mode-ui.patch18.84 KBdave reid
#2 Media_WYSIWYG_View_Mode___d7_base-13-22.png143.44 KBvegardjo
Screen_Shot_2013-12-17_at_11.21.39.png333.02 KBvegardjo
Media_WYSIWYG_View_Mode___TOSMED.png93.9 KBvegardjo

Comments

vegardjo’s picture

vegardjo commented
Issue summary: View changes
vegardjo’s picture

vegardjo commented
Title: Not respecting allowed WYSIWYG view modes » Confusing UI for WYSIWYG view modes
Category: Support request » Bug report
StatusFileSize
new Media_WYSIWYG_View_Mode___d7_base-13-22.png143.44 KB

I figured this out myself, but I'm allowing myself to keep the issue open and change it to a bug report, as I think the UI for this quite easily can be substantially clearer, and that the labels used are not accurate. There were two reasons for my original request:

  • The restrict list lists **all existing** view modes, also the ones that are not enabled on the file type. Only listing the enabled ones, or disable the checkboxes for the view modes that are not in use on the file type would be better. This is actually what the help text says aswell.
  • I'm reading the label "Restrict view modes" as "Only allow these view modes", while it actually means the opposite here. The one you check in the list are the ones that are NOT allowed, while the ones you leave unchecked are allowed. This is made more confusing as the top label / fieldset is called "WYSIWYG allowed view modes". We could change the "restrict view modes" label to something more explicit, such as "Disallowed / disabled view modes"
  • ..or, my preference "Allowed view modes" as label, and where you explicitly check the ones you would like to use in WYSIWYG, as I think that is the natural flow. Another reason for this is that if you enable a new view mode on the field, you have to actively update the settings in media to disallow it in WYSIWYG after. This is easy to forget and I feel it is more natural to actively allow them in WYSIWYG after they are enabled on the file instead.

..actually just changing what selecting a check box means (allowing in WYSIWYG, not disallowing) would solve all the above.

socialform’s picture

socialform commented

Thank you Vegardjo.. I had the same problem. And plus one for your suggestion to check the view modes that should be shown.

workplaysleep’s picture

workplaysleep commented

Thanks & Agree, very confusing, the interface allows you to choose the view modes you want to disable.
I was about to rewrite the media_wysiwyg_view_mode_form_alter hook/function...

brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented

+1 for this UI improvement. "Allowed View Modes" in which the desired modes are checked rather than the disallowed ones, would definitely be more intuitive.

erixooon’s picture

erixooon commented

+1 to allow the checked ones!
My point: Every time I (or new module) create new view mode I have to visit WYSIWYG allowed view modes and check it to disable it. And I don't like it ;-)
Thanks.

chiebert’s picture

chiebert commented

+1 from me, too - I had to put my glasses on backwards in order to set this up right...

It would also make it a lot easier to add any other dependent configuration options to this form (eg., see #2194821-41: Embedded media objects should honor display suite settings)

dman’s picture

dman commented

Yes. Every time I go to these settings, I slowly and carefully think about what I want editors to be able to do... and then set the settings entirely backwards.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Version: 7.x-2.0-alpha3 » 7.x-2.x-dev
Assigned: Unassigned » dave reid
StatusFileSize
new 2158177-media-wysiwyg-view-mode-ui.patch18.84 KB

Agreed this was confusing. I propose that we revise some wording and move this configuration to the file type edit forms. I don't think there should be a need for a separate admin page just for this, since it gets really long with lots of file types and view modes.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 2158177-media-wysiwyg-view-mode-ui.patch18.85 KB
new Selection_279.png53.17 KB

Screenshot of the new UI, on the file type page.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Related issues: +#2550409: File entities do not support entity_view_mode_prepare()

Note that this requires the current 7.x-2.x-dev of File Entity which has #2550409: File entities do not support entity_view_mode_prepare() included.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 2158177-media-wysiwyg-view-mode-ui.patch18.43 KB

Ok, removed hook_media_wysiwyg_token_to_markup_alter() too.

dave reid’s picture

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

The last submitted patch, 10: 2158177-media-wysiwyg-view-mode-ui.patch, failed testing.

The last submitted patch, 9: 2158177-media-wysiwyg-view-mode-ui.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: 2158177-media-wysiwyg-view-mode-ui.patch, failed testing.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2158177-media-wysiwyg-view-mode-ui.patch21.46 KB

This should pass tests.

  • Dave Reid committed 1e03740 on 7.x-2.x 
    Issue #2158177 by Dave Reid: Fixed confusing UI for WYSIWYG view modes...
dave reid’s picture

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

Tested and committed #17 to 7.x-2.x.

dman’s picture

dman commented

Bravo Dave!
I'll try to make some time to try it out at work today. I've got some editor UX refinements on my schedule to do I may be able to slip this into.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Related issues: +#2479947: Switch to using an 'allowed view modes' logic instead of restricting view modes

If we want to switch around how this works to an "allowed" vs "restrict" method, let's follow-up with #2479947: Switch to using an 'allowed view modes' logic instead of restricting view modes. That's a much larger change which would also require an upgrade path to rename variables.

dave reid’s picture

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

Also the committed patch doesn't show any view modes that haven't been enabled, so it matches the actual view modes available in the embed dialog (by actually using the same API function).

Status: Fixed » Closed (fixed)

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