Add the "Entity view mode" module to core

[jenlampton]

This simple module provides a lightweight UI for adding additional view modes, and provides expected template suggestion hooks. https://www.drupal.org/project/entity_view_mode

---

PR by @docwilmot: backdrop/backdrop#1645 initial port
Latest PR by @jenlampton backdrop/backdrop#1689

[ghost]

Yes please! 👍

[quicksketch]

Like other "Add X module to core" issues, I'd like to clarify that we're not planning on just adding a port of the module directly into core. The functionality from this module should be integrated directly into either the core entity.module or field.module.

[olafgrabienski]

In my opinion, custom view modes (or display modes) are especially helpful when you work with references. (Quick example: two content types, let's say Team member and Event. On the Event page you want to reference Team members who registered for the Event, but not the full information but only name and photo)

As Reference(s) is/are going to be integrated in core in 1.6, the need to build custom display modes via the UI will increase, so let's add the ability to do so in release 1.6 as well. @quicksketch @jenlampton What do you think about it?

See also @quicksketch's statement on #2084 (comment):

One feature from DS that could (and should) be implemented as soon as possible is the ability to create custom display modes.

[klonos]

This is part of #378, as in D8 they have already implemented this feature...

From #2250 (closed as duplicate of this issue):

In D6 there was Build Modes and then its successor Entity View Modes for D7 that allow you to create custom view modes through the UI. It seems that in D8 they have implemented something like that in core (or a lite version of the respective feature of Display Suite)...

They have added a new "Display modes" menu under /admin/structure that has two submenus: "Form modes" and "View modes":

"Form modes" I guess allows to manage the fields in the "Manage form display" tab (which we don't have in Backdrop - separate issue), while "View modes" allows you to manage the existing and add new view modes.

[mikemccaffrey]

It has always struck me as weird that you can't arbitrarily create new view modes in the ui, since there is so little to them.

The only tricky part is figuring out how to handle it I think the ui. Adding an interface under structure seems like a good idea. And when you create one, you can just enter the name and then check which entity types it should be available for.

[olafgrabienski]

Adding an interface under structure seems like a good idea. And when you create one, you can just enter the name and then check which entity types it should be available for.

Additionally, we could add a link "Add new custom display" on the Manage Display page of a content type, that would be under the "Custom display settings".

Side note: "Custom display (setting)" seems to be the Backdrop name for view mode, so Structure > Custom displays?

[klonos]

Adding an interface under structure seems like a good idea.

Yep, and that's what D8 users will expect.

And when you create one, you can just enter the name and then check which entity types it should be available for.

Fully agree with you there @mikemccaffrey. D8's approach is really odd: first you select from a list of links the entity type the view mode should be applied to:

...and then (no matter what entity type you have chosen in the previous page) you are requested to enter a name for it:

In this last page that requires the user to enter a view mode name, although the page title and path changes, it is (from a form point of view) basically the same for all entity types. So this 2-page wizard could as well be a single page (or dialog in our case) that allows entering a view mode name and then selecting either one entity (drop-down select or radio set) or multiple entities (checkbox set) that the view should be applied to. Not sure if the D8 approach that forces the selection of a single entity type is a technical limitation (view modes are allowed to be attached to a single entity type) or a UI implementation shortcoming (requiring selecting a single entity type in step 1 of the wizard).

In D8, they also have a page where all view modes are listed and can be edited/deleted:

...but as @olafgrabienski said, we should also have it so that people can add a view mode via a dialog without ever having to leave the "Manage display" page of the content type they are currently editing ("connect the dots" as @jenlampton says). That is more or less what I have also previously suggested over at #2205 (comment):

Side note: "Custom display (setting)" seems to be the Backdrop name for view mode, so Structure > Custom displays?

That is just the label of the fieldset that is used to hold the set of existing view modes. If you expand that fieldset, you'll see that it says "Use custom display settings for the following view modes". This is not a Backdrop thing, we inherited the same UI from D7:

...plus I think that changing the terminology is not a good thing. In D7 it was view modes and D8 has kept the same thing. They also added the "sibling" form modes that is a prelude I guess to handle the display of forms, since in D8 there's already a separate "Manage form display" tab for each content type:

[olafgrabienski]

@klonos - Re "Custom displays" vs. "View modes": You're absolutely right!

[docwilmot]

First push; need tests still.

[olafgrabienski]

Just had a look at the test site. At the moment, it's not possible to visit admin/structure/types/manage/CONTENT-TYPE/display. The link isn't in the admin bar, and if you enter such a path manually, you don't see the Manage Display page of the content type but the Configure Content Type page.

[jenlampton]

That's not a valid path. Replace the uppercase with a type name.

[olafgrabienski]

Sorry for being unclear, it was a generic placeholder. Here the two valid paths in a default installation:

• admin/structure/types/manage/page/display
• admin/structure/types/manage/post/display

[docwilmot]

Sorry to delay reply, but actually @olafgrabienski is right. There's a silly error in the code which I fixed yesterday but haven't pushed. Will do so later.

[docwilmot]

Should be fixed, including new tests.

[docwilmot]

Should also say, its not polished yet, some code comment lines need moving, and I'm not happy with the local actions code. This is up for review actually.

[jenlampton]

@olafgrabienski I checked on the bold and it's definitely in the css. I closed and reopened the PR to get a fresh sandbox.

I pushed a new PR that uses the word 'Enable' in the action link for the not-customized view modes. I think I like this better. It's not obstructionist by hiding the action, but is also less inviting than the word 'Customize' which doesn't communicate to the user that it hadn't been "customized" already.

I also added the bundle label in the Customized / Not customised headings, I thought that would help make it clear that different view modes could be customized (or not) for different content types.

I changed the action link back to simply Add view mode which I also like better - it's more inline with the other action links in core.

edit: I also removed the select list from the Add view mode form, and added the final tests to ensure that default settings are properly assigned to new (and newly enabled) view modes.

[jenlampton]

@quicksketch I think this is ready for a code review - the feature is close enough that we can iterate on UI changes after Jan 1st - but I want to make sure you are on board with where the code was added (field UI module) and what it looks like.

[quicksketch]

Posted a hella code review to the PR: backdrop/backdrop#1689 (review)

Overall, in concept everything looks great, though there are tons of typos and code style recommendations.

The big things that need addressing code-wise:

• Sometimes a view mode has the property custom settings and sometimes custom_settings. This inconsistent use of underscores is terribly confusing. Is this something we've inherited, or can we fix this?
• We don't support renaming most items that have machine names (vocabularies, menus, image styles, etc) because handling the rename is always messy and frequently error-prone. We should remove renaming ability for custom view modes. It looks like renaming is disabled in the UI, but there is still a ton of code related to handling renames.
• I'd like to get entity_preprocess() removed and just fix the places where we are rendering entities to include the view mode suggestion directly, rather than altering it in.

[klonos]

@jenlampton you rock! I feel that everything is so much better in this new iteration for all the reasons that @quicksketch mentioned.

May I suggest a change in the help text and please feel free to ignore as I'd hate to start bike-shading over this issue further...

View modes are variations of how a [piece of content|user profile|comment|taxonomy term|...] can be displayed. Each view mode can be configured to have different fields shown or hidden. Also, the same field can be configured to be displayed in a different format per view mode.

I feel that this help text is better than the current one for the following reasons:

• The term "view mode" is at the start of the help text instead of the end of it (less risk that the user considers this text unrelated and won't read it).
• Small sentences make it easier to understand things.
• Each sentence after the 1st definition one actually describes what the "Manage display" button does. This way the user understands what view modes can be used for.
• I find "are often displayed differently" very vague and vague is not good for documentation/help. I feel that the suggested help text fixes that by explaining what "displayed differently" is (show/hide fields or display them in a different format).

[jenlampton]

Sometimes a view mode has the property custom settings and sometimes custom_settings. This inconsistent use of underscores is terribly confusing. Is this something we've inherited, or can we fix this?

I totally agree. Unfortunately both instances are already in core. We can change one, but I'm not sure what the ramifications of doing so would be. See underscores in field_view_mode_settings() vs spaces in entity_get_info(). Maybe we create a follow-up issue? (see #2449)

We should remove renaming ability for custom view modes. It looks like renaming is disabled in the UI, but there is still a ton of code related to handling renames.

I tried to kill it in the most recent PRs. I'll double check and see if I missed any.

I'd like to get entity_preprocess() removed and just fix the places where we are rendering entities to include the view mode suggestion directly, rather than altering it in.

okay. I'll try that now.

[jenlampton]

Thank you @klonos @docwilmot @herbdool and @quicksketch for the reviews and feedback! I've addressed all the comments and pushed a PR: backdrop/backdrop#1689

[olafgrabienski]

I like the new iteration very much, thank you for working so long on it!

Maybe I found another small bug: After having added a view mode for Posts, I get the following page:

• Title: Access denied
• Status message: The view mode Yet another can now be customized.
• Text: You are not authorized to access this page.

Afterwards, I was however able to customize the new view mode.

[jenlampton]

Thanks @olafgrabienski, can you please list the steps to reproduce the problem?

[olafgrabienski]

@jenlampton I can't reproduce the issue anymore. Anyway, these were the steps:

• Go to Home > Administration > Structure > Content types > Post.
• Choose the link Add view mode.
• Enter a View mode label.
• Hit the Save button.

[jenlampton]

Thanks! I may have fixed it with the last push of the PR. :)

[olafgrabienski]

@jenlampton I had a look at the DBlog where I found a message related to the "Access denied" error, see http://1689.backdrop.backdrop.qa.backdropcms.org/admin/reports/event/74.

At the same time there was a php notice. When you have a look at the DBlog, you'll see that some PHP notices appear also later. I guess these happened when I tried to reproduce the "Access denied" error.

[quicksketch]

There are some failing tests related to testing the old Field UI in FieldUIManageDisplayTestCase. I'll work on resolving these failures.

I also noticed that now we have restored the "Enable" link on the listing page, that these links are not CSRF protected by a token like other similar links. I can add a token and check to the access check as well.

[quicksketch]

Okay I added a commit to the PR at backdrop/backdrop#1689. If all tests are passing, we're finally good to go on this one I think.

@olafgrabienski It appears the access denied problems may have been caused by the entity_view_mode_exists() function throwing a PHP notice because of an undefined variable. The tests were also failing on this point, but it should be fixed now.

[jenlampton]

Hm, it looked like tests were failing. I closed/opened the PR to run them again.

[jenlampton]

BACKFLIPS

[quicksketch]

This has been merged into 1.x for 1.6.0. THANK YOU everyone for the help on this issue. This was probably one of our most bikesheddy issues to date, but we arrived at a solution with which everyone seems satisfied.