When #2029321: Provide list and form controllers for EntityViewMode and EntityFormMode lands.

Current text in the UI with #25 applied.

Manage > Structure > Display Modes

Manage > Structure > Display Modes > Form Modes

Manage > Structure > Display Modes > Form Modes > Add new form mode

Manage > Structure > Display Modes > Form Modes > Edit (editing one form mode)

Manage > Structure > Display Modes > View Modes

Manage > Structure > Display Modes > View Modes > Add new form mode

Manage > Structure > Display Modes > View Modes > Edit (editing one form mode)

Files: 
CommentFileSizeAuthor
#64 interdiff-56-63.txt7.24 KBbatigolix
#64 entity-hook-help-2041819-63.patch6.74 KBbatigolix
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,851 pass(es).
[ View ]
#56 entity-hook-help-2041819-56.patch2.33 KBpgautam
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,732 pass(es).
[ View ]
#52 entity-hook-help-2041819-52.patch2.94 KBjhodgdon
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,789 pass(es).
[ View ]
#50 entity-hook-help-2041819-50.patch2.87 KBer.pushpinderrana
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,641 pass(es).
[ View ]
#48 entity-hook-help-2041819-48.patch2.93 KBjamesquinton
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,507 pass(es).
[ View ]
#43 entity-hook-help-2041819-43.patch2.9 KBjamesquinton
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,501 pass(es).
[ View ]
#35 entity-hook-help-2041819-35.patch2.25 KBjamesquinton
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,270 pass(es).
[ View ]
#32 2041819_entity-view-modes-list.png14.62 KBliquid06
#32 2041819_entity-view-mode-edit.png14.12 KBliquid06
#32 2041819_entity-view-mode-add.png18.51 KBliquid06
#32 2041819_entity-form-mode-list.png18.73 KBliquid06
#32 2041819_entity-form-mode-edit.png14.03 KBliquid06
#32 2041819_entity-form-mode-add.png15.2 KBliquid06
#32 2041819_entity-display-mode.png18.23 KBliquid06
#25 entity-hook-help-2041819-25.patch2.16 KBmartin107
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,753 pass(es).
[ View ]
#25 interdiff-21-25.txt1.7 KBmartin107
#21 entity-hook-help-2041819-21.patch2.25 KBfilijonka
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,418 pass(es).
[ View ]
#20 entity_help-helpPage.png146.9 KBJohn_B
#20 entity_help-form.png100.03 KBJohn_B
#16 entity-hook-help-2041819-3.patch2.38 KBdavid.czinege
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,818 pass(es).
[ View ]
#8 entity-hook-help-2041819-2.patch2.31 KBMarc DeLay
PASSED: [[SimpleTest]]: [MySQL] 58,073 pass(es).
[ View ]
#5 entity-hook-help-2041819-1.patch5.15 KBMarc DeLay
PASSED: [[SimpleTest]]: [MySQL] 58,181 pass(es).
[ View ]
#4 entity-hook-help-2041819.patch3.68 KBMarc DeLay
FAILED: [[SimpleTest]]: [MySQL] 58,094 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

swentel’s picture

Marc DeLay’s picture

creating a hook_help function in entity.module.

edit 5 pm local time:
I have most of this done. I have a couple questions about possible entity types and managing display modes. I will return to this in the morning.

pfrenssen’s picture

@Marc DeLay, can you post your work in progress? Don't be shy, even if it is not fully finished, I would love to take a peek :)

Marc DeLay’s picture

StatusFileSize
new3.68 KB
FAILED: [[SimpleTest]]: [MySQL] 58,094 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here's what I have at the time. I basically copied the content of entity_menu in a new function entity_help and I am going through url by url and adding the messages.

I got stuck a bit at the case 'admin/structure/display-modes/view/manage/%' :
switch, but I think That part is okay-ish now. Working through he rest this morning.

Marc DeLay’s picture

Status:Active» Needs review
StatusFileSize
new5.15 KB
PASSED: [[SimpleTest]]: [MySQL] 58,181 pass(es).
[ View ]

Here is my attempt at a full hook_help function for the entity module.

The one outstanding question I have is different kind of entities that can have display modes. Can we create whole new entity types outside of node, user, comment, taxonomy, blocks? There is a switch in the help module to try to point the user towards the place where they can actually effect the output of a display mode. it's hard coded for just the default entity types right now. Furthermore, if new entity types are possible it doesn't seem like we would be able to know where to send a user for the actual display mode management.

I feel like this whole display mode management section is a bit misleading about being able to actually effect the display modes, when you can really only create, delete and rename them from here.

tl;dr; - is it worth trying to push the user towards the myriad of places where they should actually be going to manage display modes in an end-user sense?

pfrenssen’s picture

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+/**
+ * Implements kook_help()
+*/

Typo: should be 'hook_help()', and should end in a period. The closing "*/" should also be indented with a space so the stars line up vertically.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+  switch($path) {
+    case 'admin/help#entity':

Language constructs such as switch, foreach, if, etc should have a space between the word and the opening parenthesis, so it can be distinguished from function calls.

Will continue review later, I have been summoned for a photo session.

pfrenssen’s picture

I finished the review.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+      $output = '<h3>'.t('About').'</h3>';

The concatenation operator (period) should be preceded and followed by a space. This should be done throughout the whole code.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+      $output .= '<p>'.t('The entity module controls the display modes for entities. The interface provided by the module is only for creating, deleting and naming of display modes. The management for the output of display modes is handled in the admin section of the content types they are assigned to.').'</p>';

You don't need to mention the sentence about the output of the view modes, as this is handled by the (optional) Field UI module, and is documented in the help files for Field UI.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+      $output .= '<p>'.t('You can edit user\'s permission to manage display modes on the <a href="@permissions_link">permissions page</a>.', array('@permissions_link' => '/admin/people/permissions')).'</p>';

I would use "user permissions" rather than "user's permission". You can also link directly to the permissions for the Entity module: /admin/people/permissions#module-entity.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes' :

Don't put a space before the ending double colon in case statements.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/view' :

Maybe put an empty line before each case statement? It might improve readability.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/view/list' :
+    //    I don't think this has an actual page?

No, you're right, this page does not exist. You may remove this section.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/view/add' :    ¶

This line has trailing whitespace.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/view/add/%' :
+      return '<p>'.t('Choose a label for the new display mode for a @type.', array('@type' => str_replace('_', ' ', $arg[5]))).'</p>';

This interface is obvious: it only contains a label field and a save button. I wouldn't bother to provide a help text here.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/view/manage/%' :
+      $arg_list = explode('.', $arg[5]);
+      $entity_type = $arg_list[0];
+      switch($entity_type) {
+        case 'node' :
+          $edit_option = ' '.t('Detailed settings for node display modes can be found in the <a href="/admin/structure/types">content type admin</a> section.');
+          break;
+        case 'user' :
+          $edit_option = ' '.t('Detailed settings for user display modes can be found in the <a href="/admin/config/people/accounts/display">account settings</a> section.');
+          break;
+        case 'taxonomy_term' :
+          $edit_option = ' '.t('Detailed settings for taxonomy display modes can be found in the <a href="/admin/structure/taxonomy">taxonomy admin</a> section.');
+          break;
+        case 'comment' :
+          $edit_option = ' '.t('Detailed settings for comment display modes can be found in the <a href="/admin/structure/types">content type admin</a> section.');
+          break;
+        case 'custom_block' :
+          $edit_option = ' '.t('Detailed settings for block display modes can be found in the <a href="/admin/structure/custom-blocks/types">custom block admin</a> section.');
+          break;
+      }
+      return '<p>'.t('You can edit the display mode name or delete the display mode.').$edit_option.'</p>';

This section is problematic as you already recognized. It is indeed possible to create new entities. Keeping a hardcoded list is indeed not a good solution.

I would just get rid of this section. This interface is very simple and does not really need a help text.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/view/manage/%/delete' :

This section is not needed. There already is a text provided on this page that basically says the same thing.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/form/list' :
+//    I don't think this has an actual page?

This can indeed be removed.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/form/add/%' :
+      return '<p>'.t('Choose a label for the new form display mode for a @type.', array('@type' => str_replace('_', ' ', $arg[5]))).'</p>';    ¶

Same remark as above, this interface is too simple to require a help text.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/form/manage/%' :
+      $arg_list = explode('.', $arg[5]);
+      $entity_type = $arg_list[0];
+      switch($entity_type) {
+        case 'node' :
+          $edit_option = ' '.t('Detailed settings for node form display modes can be found in the <a href="/admin/structure/types">content type admin</a> section.');
+          break;
+        case 'user' :
+          $edit_option = ' '.t('Detailed settings for user form display modes can be found in the <a href="/admin/config/people/accounts/form-display">account settings</a> section.');
+          break;
+      }
+      return '<p>'.t('You can edit the display mode name or delete the display mode.').$edit_option.'</p>';

Same as above. A hardcoded list is not a good solution, and the interface is simple and doesn't need a help text.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+    case 'admin/structure/display-modes/form/manage/%/delete' :
+      return '<p>'.t('Deleting a display mode can not be undone. If a form was using this display mode it will use the default display mode instead.').'</p>';

There already is a help text on this page so this is not necessary.

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,75 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+}
\ No newline at end of file

Make sure you end the document on a newline.

Thanks a lot for the awesome work so far! If you plan on continuing work on this, may I suggest you take a look at the official Drupal coding standards guide? It explains how whitespace is handled, and all code needs to conform to these guidelines before it can be added to core.

Marc DeLay’s picture

StatusFileSize
new2.31 KB
PASSED: [[SimpleTest]]: [MySQL] 58,073 pass(es).
[ View ]

Sorry about all the messed up spacing and stuff.

Fixed the typos and removed the unneeded statements.

pfrenssen’s picture

+++ b/core/modules/entity/entity.moduleundefined
@@ -159,3 +159,40 @@ function entity_entity_bundle_delete($entity_type, $bundle) {
+      $output .= '<p>' . t('The entity module controls the display modes for entities. The interface provided by the module is only for creating, deleting and naming of display modes.') . '</p>';

Maybe rewrite the second sentence? Now that the third sentence is gone, the "only" seems a bit misplaced. Something like "The module provides an interface for creating, deleting and renaming display modes." ?

Sidenote: I am more used to the term "view modes" than "display modes", I'm not sure why sometimes one is used and sometimes the other.

The rest looks great to me. I am not a native English speaker though so maybe someone more proficient in English should take a gander as well.

pfrenssen’s picture

Status:Needs review» Needs work
Marc DeLay’s picture

My thinking here, back to the patch with the nested switches, is that this interface is quite confusing.
It seems to indicate that you are here to configure the way nodes, blocks, forms etc are displayed. The problem is that you can't control the actual display part of the display modes from here. You can 'only' create, delete and rename display modes.

edit - upon further review of the issue that created this interface https://drupal.org/node/2029321
They wanted the system out the door without worrying about the UI/UX implications. I think that I was closer with the second patch. User's will see "manage display modes" and come here expecting to manage the way things are displayed. We need to be prepared for those users.

I would suggest that the help text should be hinting where users should be looking to 'manage the modes of display".

Bojhan’s picture

I fully agree with Marc here, it'd be nice toh ave some screens here to review.

pfrenssen’s picture

I think the inline help should be about explaining the current interface, and should not lead the user away from the task at hand.

I'm in favor of linking to the display mode management screens, but this should be done in the action buttons on the overview, definitely not in the help system which may or may not be enabled. The help system is not intended for navigation purposes.

There are now "Edit" and "Delete" actions, we can add a "Manage" action. That is out of scope for this issue though.

yoroy’s picture

Still needs screenies :-)

martin107’s picture

Issue summary:View changes
Issue tags:+Needs reroll

entity-hook-help-2041819-2.patch no longer applies.

david.czinege’s picture

StatusFileSize
new2.38 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,818 pass(es).
[ View ]

Hello. I am new in Drupal patching. I created a re-rolled patch for this issue. Please review it.

david.czinege’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
catch’s picture

Issue tags:+Novice

Tagging novice to hopefully encourage reviewers.

martin107’s picture

@david.czinege Hi... welcome to the community

This is a very very minor point but I hope it is useful information

Can I recommend installing dreditor

http://drupalize.me/videos/installing-and-using-dreditor
https://dreditor.org/

If you look at the patch with this it will show some red marks highlighting some white space errors.

Alternatively

if you

git apply entity-hook-help-2041819-3.patch

you will see line number point to them.

John_B’s picture

StatusFileSize
new100.03 KB
new146.9 KB

Patch in #16 applies to 8.0-alpha11 with warning:

entity-hook-help-2041819-3.patch:9: trailing whitespace.

warning: 1 line adds whitespace errors.

I have uploaded a couple of screenshots. Looks fine to me. I have also compared the code of the patch in #8, which applies to 8.0-alpha3 but not later versions. the earlier patch does not appear to include anything useful which is missing from the patch in #16.

filijonka’s picture

StatusFileSize
new2.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,418 pass(es).
[ View ]

just cleanup patch sp it fits the coding standard

fago’s picture

Priority:Critical» Major

I do not think this qualifies as critical, so demoting.

filijonka’s picture

jhodgdon’s picture

Status:Needs review» Needs work

hook_help has changed. It now uses routes and not paths. Change record: https://drupal.org/node/2250345

This patch need to be updated.

martin107’s picture

Status:Needs work» Needs review
StatusFileSize
new1.7 KB
new2.16 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,753 pass(es).
[ View ]

Updated patch as per #25.

I have visually inspected relevant pages and can confirm the appropriate text appears when required.

jhodgdon’s picture

Status:Needs review» Needs work

This patch has some problems:

a) These are called "display modes" not "view modes", correct? In which case:

+    // View modes.
+    case 'entity.view_mode_list':
+      return '<p>' . t('Manage custom view modes. You can create, rename or delete display modes from here.') . '</p>';

The code comment here is not necessary (IMO), and the text should say "display modes" not "view modes".

b)

+    case 'entity.view_mode_add':
+      return '<p>' . t('First pick which entity type you want a new display mode for.') . '</p>';

This IMO is not useful help. If the UI doesn't already tell you you need to pick an entity type, then the UI needs a fix. Putting this kind of help at the top of the page is not a solution to a bad UI, and if the UI is good, then this help telling you what to do is not necessary.

The help at the top of a page should be a conceptual explanation, not an explanation of how to use the UI.

c) So... We have "display modes" and "form display modes" then? Really? OK, then:

+    case 'entity.form_mode_add':
+      return '<p>' . t('First pick which entity type you want a new form display mode for.') . '</p>';
+
+    case 'entity.form_mode_edit':
+      return '<p>' . t('You can edit the display mode name or delete the display mode.') . '</p>';

This last one should say "form display mode" not just "display mode" then.

d) Really... Most of this help is probably not necessary. It is not necessary to explain things that are obvious (or should be) in the UI, and the text here doesn't really tell you anything that might not be obvious, like what a "display mode" is in the first place, what they're used for, why you would want to create one, etc.

webchick’s picture

"the text here doesn't really tell you anything that might not be obvious, like what a "display mode" is in the first place, what they're used for, why you would want to create one, etc."

That's exactly where this patch could be most useful, IMO. "Display modes," "View modes," etc. are all Drupal jargon, and will be totally unfamiliar to new users. What are they, what do they mean, why would I use them? I can tell by the fact that this admin interface looks like other admin interfaces that I can add, edit, and delete things here. Tell me more about the things. :)

Example: "Display modes flibber your gibbets and are useful for smuggery buggery."

Obviously with real words instead of made-up ones. ;) But just a single sentence to give me a gist.

If longer/more involved descriptions are needed, "Display modes" could also link to the standard hook_help() text that describes these overall concepts, similar to what taxonomy_help does.

delzhand’s picture

Status:Needs work» Needs review
markie’s picture

Status:Needs review» Reviewed & tested by the community

patch successfully applied, but I am not seeing the help messages. But then maybe I am looking at it wrong.

Bojhan’s picture

Status:Reviewed & tested by the community» Needs work

This needs work, per webchick's comment.

liquid06’s picture

I'm a newbie at the Austin sprint. :) Since it looks like the patch is working OK for people, I'll add some screenshots of that proposed state so other reviewers can quickly get an idea of what the UI is here.

liquid06’s picture

Title:Create hook_help for display modes» Update hook_help to describe display modes
Issue summary:View changes
StatusFileSize
new18.23 KB
new15.2 KB
new14.03 KB
new18.73 KB
new18.51 KB
new14.12 KB
new14.62 KB

I'm unfamiliar with view modes as implemented in Drupal 8. Even for those who have worked with Drupal 7, it might be helpful to incorporate something in the help text about the difference between the types of modes.

When faced with a choice of View mode or Form mode, having clicked on Display modes and not quite being sure what that means in the first place, it's compounding confusion.

I'm updating issue metadata based on my understanding of #26 and #27

jhodgdon’s picture

liquid06: That is extremely valuable and excellent feedback! Yes, let's make sure to explain exactly what display and form modes are, at least on their respective landing pages. And I agree on the Structure page, an explanation would be useful there too!

That is pretty much also what webchick and I said in #26/#27 and why this issue is still at "needs work". Let's get it fixed!

jamesquinton’s picture

Assigned:Unassigned» jamesquinton
jamesquinton’s picture

Assigned:jamesquinton» Unassigned
Status:Needs work» Needs review
StatusFileSize
new2.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,270 pass(es).
[ View ]

Hi!

I've updated the copy, which hopefully explains view/display modes better.

  • 'Display mode', 'View mode' and 'Form mode' are used more consistently, which I think helps.
  • On the add/edit views, I don't think it needs as much explanation - as I would assume the user already knows what it is by this point.
jhodgdon’s picture

Title:Update hook_help to describe display modes» Update hook_help and menu entries for Entity module to describe display modes
Status:Needs review» Needs work

Thanks! This is moving in the right direction of trying to explain what view and form modes are... Some things to think about and/or fix:

a)
...render an entity in different contexts, i.e \'Full content\' or \'Teaser\'.
This is incorrect usage -- i.e. means "that is". You meant "e.g." here. But everyone is always confused about i.e. and e.g., and they are nearly never punctuated correctly either, so it's far preferable to write out "for example". The correct punctuation would then be: ...contexts; for example, ...

b) Help text for pages should not describe obvious UI elements of the page. So please take out text like "Manage ... on this page" -- if they are on an admin page with page title "Whatever", we do not need help saying "This page is where you manage Whatever on your site". It's obvious. We also don't need help saying "The fields below are for X and Y" since the field titles should tell them that. So if that leaves the help text empty, we do not need any help text. See: https://drupal.org/node/604342 for more information about our UI standards.

For example:

+    case 'entity.view_mode_add':
+      return '<p>' . t('Pick an entity type for your new view mode.') . '</p>';

That is not necessary or useful help.

c) "Form modes allow you to render the same form in different contexts" ... uh? . what does that mean? I think we should be able to come up with a better description. I'm also not all that thrilled by the display/view mode description. Can we make up something better please? Maybe something like "different ways to display the content for different purposes" and "different editing forms for the content for different purposes"? I think I would not talk about "rendering" at all, and I am not sure I would talk about "contexts" either... This help is for novice users who don't have a clue what these things are, so we need it to be pretty simple and very clear.

d) There is still one piece of help about "display modes" and another about "view modes". I think we still need to pick one and stick with it. From the screen shots on this issue, it looks like it should always say "view modes" and never "display modes", right?

e) Go back to the issue summary. This issue also needs to fix the menu entry descriptions for view modes and form modes, not just the hook_help.

Thanks!

jamesquinton’s picture

Assigned:Unassigned» jamesquinton

Hi @jhodgdon!

Thanks for the feedback - reassigning myself.

a) Interesting, I'll fix that!
b) Agreed.
c) Good point. How about "Display modes allow you to display content differently, in different situations." (This can be adapted for form modes)
d) I thought this too, however, they are different things. A display mode is an umbrella term that covers view modes (for entities) and form modes (for forms). The help text refers to the same term as the UI. "Display modes" is the correct term for the 1st screenshot.
e) Good point, although let's get the copy agreed on first.

jhodgdon’s picture

RE #37

(c) OK... maybe "for different situations" and leave out the comma?

(d) OK. Then in that case the explanatory text for Display Modes should not read the same as the text for View modes, but should instead say something like "Display modes include both View modes [insert explanation] and Form modes [insert explanation]". Right?

yoroy’s picture

Thanks for working on this & good review feedback as well. One suggestion:

C. "Use display modes to display content differently for different situations."
I would try to stay away from "allow you to", sounds more restrictive than empowering. I'd like people to feel empowered :-)

jamesquinton’s picture

@jhodgdon - Thanks! Agree with both!

@yoroy - "enable you to"?

yoroy’s picture

I mean to suggest to change

Display modes allow you to display content differently, in different situations.

to

Use display modes to display content differently for different situations.

jhodgdon’s picture

I prefer Yoroy's wording also. Saying "Use xyz to accomplish foo" is much more engaging than "foo allows you to accomplish xyz".

jamesquinton’s picture

Assigned:jamesquinton» Unassigned
Status:Needs work» Needs review
StatusFileSize
new2.9 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,501 pass(es).
[ View ]

Thanks @yoroy and @jhodgdon for your input. Implemented in the attached patch.

jhodgdon’s picture

Status:Needs review» Needs work

Thanks for the patch!

So... I am still concerned that the Display Modes page help says "Configure display modes to display content differently for different situations" but from what you said in #37, "display modes" actually means both view modes (which is what is described here) and form modes (which aren't). The examples of "full content" and "teaser" are also specific to view modes.

So I think the two Display Modes descriptions should probably say something like "Display modes include view modes (explanation) and form modes (explanation)". Right?

I also wonder if this explanation for forms:

Configure form modes to display forms differently for different situations

should say something about entities or be specific about the forms being for adding/editing content? Because this description sounds like you could display any form differently using a form mode, and that is not the case.

jamesquinton’s picture

Assigned:Unassigned» jamesquinton

Sure - although tbh that could start getting a little long on the Display Modes page? If you don't see that as an issue, then great!

Will update the Form mode help.

giangnguyenchuc’s picture

I think so.

jhodgdon’s picture

I would rather see accurate help than short help that is not accurate.

jamesquinton’s picture

Assigned:jamesquinton» Unassigned
Status:Needs work» Needs review
StatusFileSize
new2.93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,507 pass(es).
[ View ]

Updated the Display modes help text.

Looking closer at form modes, you can add these for various different forms (user, taxonomy, content) etc - I can't think of anyway to be more specific without it being in-accurate.

jhodgdon’s picture

Status:Needs review» Needs work

They are all entity add/edit forms. We do need to be more specific, because you cannot alter any other of the many types of forms in Drupal.

Also... I've been bothered by this for several reviews: I think it's weird to say that you are making different "displays" of a form. I think it would be better to say something like "create different forms to edit entities for different situations" than what is there now?

Also:

+++ b/core/modules/entity/entity.menu_links.yml
@@ -1,15 +1,15 @@
entity.display_mode:
   title: 'Display modes'
-  description: 'Configure what displays are available for your content and forms.'
+  description: 'Configure display modes to display content differently for different situations; for example, Full content or Teaser.'

This still needs the fix for display modes, see previous comments.

And the text for display modes:

Display modes include view modes (use view modes to display content differently for different situations) and form modes (use form modes to display forms differently for different situations).

How about:

Display modes include view modes (used to content differently for different situations) and form modes (used to make different entity editing forms for different situations).

er.pushpinderrana’s picture

Status:Needs work» Needs review
StatusFileSize
new2.87 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,641 pass(es).
[ View ]

I gone through from all posts of this thread and found it is coming closer. I am here just incorporating above suggested changes in #48 patch.

Attached is rerolled patch.

jhodgdon’s picture

Status:Needs review» Needs work

Sorry, but this patch is still not right. Basically, please read #49 again. What I pointed out there is still a problem.

jhodgdon’s picture

Status:Needs work» Needs review
StatusFileSize
new2.94 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,789 pass(es).
[ View ]

I think this would be more efficient if I just made a patch. An interdiff is not really useful here since every line in the patch changes.

jamesquinton’s picture

Status:Needs review» Needs work

TBH I don't think 'Configure form modes to edit content differently' is quite right - as you can also use the forms to add. Also these forms include user forms and comments etc, which may not be obvious if 'content' is used.

jhodgdon’s picture

Fair enough. Please make a new patch. I am out of ideas. :)

Berdir’s picture

Issue tags:+Needs reroll

This moved to field_ui.module, so will need to be re-rolled/updated.

pgautam’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new2.33 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,732 pass(es).
[ View ]

Rerolled patch #52.

swentel’s picture

+++ b/core/modules/field_ui/field_ui.module
@@ -56,6 +56,17 @@ function field_ui_help($route_name, RouteMatchInterface $route_match) {
+    ¶

Spaces here,

jhodgdon’s picture

We apparently lost the help we used to have on the Entity module for what entities are. :(((((( This also needs to be fixed. admin/help/field has a link to the entity module help, but it's gone.

So... Can we do that here or do we need another issue? And who thought it was OK to remove the Entity module without moving its hook_help() documentation somewhere??!?

The patch where the entity help was created was #2091403: Create hook_help for Entity module

realityloop’s picture

Issue tags:+Amsterdam2014, +Needs reroll
batigolix’s picture

Issue tags:+documentation, +sprint
Cottser’s picture

Issue tags:-Needs reroll
batigolix’s picture

added a follow-up issue to reroute links from different hook_help texts to the entity module:
#2349405: Update hook_help texts that refer to the entity module

batigolix’s picture

Assigned:Unassigned» batigolix

i'm gonna add the missing part that explains entities to the field_ui module's hook_help text

batigolix’s picture

StatusFileSize
new6.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,851 pass(es).
[ View ]
new7.24 KB

Patch adds the explanation about entities that used to be in the entity.module's hook_help()

And it removes the references to the field_ui help page.

batigolix’s picture

Assigned:batigolix» Unassigned
batigolix’s picture

Status:Needs review» Closed (won't fix)

I found we already had an issue that deals with the Field UI module's help text: #2091321: Update hook_help for Field and Field UI module

So I think that in this particular module, we should not propose any changes to the Field UI module.

According to the title of this issue we should deal here with the entity module (which does not exist anymore). So as far as I can see we can close this issue because the task has become obsolete.

batigolix’s picture

No need to rush this, so I *propose* to Close this issue and continue the work here: #2091321: Update hook_help for Field and Field UI module

batigolix’s picture

Status:Closed (won't fix)» Active
jhodgdon’s picture

Status:Active» Closed (duplicate)

I think that is a fine idea, let's just close this as a duplicate.