Background:
This issue is part of the task to update the hook_help texts of the Drupal 8 modules:
#1908570: [meta] Update or create hook_help() texts for D8 core modules

Tasks:
- review / write the hook_help text according to help guidelines

Beta evaluation: This is unfrozen -- documentation / UI text.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sree’s picture

Assigned: Unassigned » Sree

Assigning ...

Sree’s picture

FileSize
19.44 KB

While working on this I found some UI related misalignment:

When we visit taxonomy admin page : /admin/structure/taxonomy just after we install Drupal there is alignment problem.
'Operations' dropdown gets displayed under 'Weight' heading instead of getting displayed under 'Operations' heading.
Once when we add a new Vocabulary this gets auto adjusted.

Please check the screen shot attached: See attached screen grab

Please let me know if we need to raise a separate issue to address this problem or should we patch it here itself?

Thanks,

Sree’s picture

Current help doesnt provide any information about the operations dropdown, show/hide row weights available at /admin/structure/taxonomy.

Can I add that in the patch?

lostkangaroo’s picture

I would file #2 under a separate issue since the two are unrelated.

jhodgdon’s picture

RE #2, yes please file a separate issue in component taxonomy.module. If you can make a patch, even better! Also, looking at the screen shot, what is that extra "+" button above the table supposed to be, next to the "Add Vocabulary" button? Maybe you can put that in your separate issue too?

RE #3, yes please add something about the operations to the help. And the show/hide row weights, which should be combined with the places in the help where we describe how to change the order of taxonomy terms in a vocabulary and the order of vocabularies (explain that you can drag/drop or use the show/hide row weights link/button to do it with accessible select lists instead of your mouse).

Actually, I have no idea what the weight is used for in taxonomy vocabularies anyway. In Drupal 6 it would I think determine the order of taxonomy fields on a node edit screen, but since Drupal 7 these are fields so it is not relevant. Do you know what weight of vocabularies is for? If you can figure that out, it should probably be explained in the help. Weight/hierarchy of taxonomy terms is definitely relevant, because it determines the order in drop-down lists for selecting terms.

Sree’s picture

Thanks for the quick clarifications. Heres the new issue for #2: https://drupal.org/node/2104517.
I would start working on this new issue once I am done with the current patch.

Coming to the '+' button I will check the details about it & will see if it has any particular significance in any scenario or not &would act upon it accordingly.

Weight in Drupal 7 seems just adjusting the order... I ll look into more specific features of it& would update you about it so that we can decide upon it accordingly.

jhodgdon’s picture

I agree, the weight on the Vocabularies is adjusting the order on the Vocabularies listing page. I guess that even though I don't think the order is used anywhere else, it's still useful to be able to organize that page.

Sree’s picture

'+' button is pointed to same link as ' + Add Vocabulary ' button. These buttons are used to add new vocabularies.
So, we need to remove this redundant '+' button as well.
Here is the issue for this: https://drupal.org/node/2107117

Sree’s picture

Status: Active » Needs review
FileSize
1.89 KB

Updated a small description about 'Operations' dropdown & 'Show row weights'

jhodgdon’s picture

Status: Needs review » Needs work

Sree: Thanks for all the testing and issue filing! That is a very valuable contribution to making Drupal 8 viable.

Regarding this latest patch... Our philosophy in hook_help is that the About section gives an overview of what the module is for and defines terminology. Then the Uses section gives details of things you can do and how to do them. So I think that the information about row weights and operations should be in Uses.

Sree’s picture

Hi Jennifer,
Thanks for your valuable comments & pointing me to the correct direction. I ll update the patch accordingly & would re-submit it for further review.

Sree’s picture

Status: Needs work » Needs review
FileSize
2.27 KB

Updated in Uses section ...

batigolix’s picture

Status: Needs review » Needs work

The current help text does not mention the possibility to add fields to taxonomy terms.

We would need a template text for all fieldable entity types that can be used in the help text of all entity types to explain this possibility, with links to both field and field_ui help text, i think.

ifrik’s picture

@batigolix: We can use the link module as a template. That has the agreed wording for the About section and for the first Use in it, and has been submitted to core. So if you got the latest 8.x (or in any case the next beta version to be released next week) then you got that text at hand.

batigolix’s picture

I suppose we could but this is a slightly different situation. The link field help text is good as template for other field module. Taxonomy term is the container for fields, like the content type and the user. In D8 we will have +/-8 of such containers, so 1 template text could be useful

batigolix’s picture

Assigned: Sree » Unassigned

unassign

jhodgdon’s picture

Yeah, we don't want to use the Link module as a template -- that's a field-providing module, not an entity-providing module. However, we do have text in the Node module help about fields already. Let's use that as a template, if possible. Thanks for noticing that batigolix!

We may also have text in the User module about fields; if not, that should be added too, and also to Comment. Can someone make sure those issues are updated/added? (If the User and Comment modules have already been marked Fixed, lets' start with new issues if they don't already talk about fields in a similar way to what we're doing in Node.) Any other modules that need this help?

jhodgdon’s picture

We just had a change to hook_help, on this issue: #2183113: Update hook_help signature to use route_name instead of path.

Here is the change record: https://drupal.org/node/2250345

This patch will need a reroll for this change.

mparker17’s picture

This is a straight re-roll: no changes so no interdiff.

mparker17’s picture

Status: Needs work » Needs review
FileSize
2.27 KB
2.27 KB

There was a tab (whitespace error) in the original patch in #12, so here's one that fixes it.

There was also a spelling error ("reaarange" -> "rearrange").

Feedback welcome.

jhodgdon’s picture

Status: Needs review » Needs work

OK, thanks for the reroll and typo fix!

So I went back to the previous comments on this issue... There is still more than needs to be done in the Taxonomy module help.

a) In the About section, it should mention something about providing a field, instead of saying " assign the vocabularies to content types", which is not even accurate in Drupal 7, much less Drupal 8.

b) The About section's link to the drupal.org page is not following standards.

c) All links to drupal.org should be https.

d) All URLs should use !var not @var in t().

e) In the "Creating vocabularies" Uses item, it introduces terminology "controlled vocabulary". Unless this term appears in the Taxonomy module UI somewhere (and I do not think it does), we should not introduce it in the help.

f) Is that accurate that a term can have multiple parents? Really? How? I cannot see how that would work within the drag-and-drop interface, so if it is possible it should be documented how you would do that (like, if you click Edit on a term? or ???) because I'm sure you cannot do it by just drag-and-drop.

g) Then I get to the free-tagging vocabulary definition, and I think maybe this appears in the UI but we should look and make sure it is still called that.

h) In the patch, I don't think we need to describe "show weights" as it is not a function of the Taxonomy module, but a generic Drupal UI thing.

i) We also do not need to describe things that are obvious in the UI, like how to drag things around, the Operations list, etc. (We can generically say something like "On the page for each vocabulary, you can ... " but we don't really have to say "It's in the Operations area, duh".)

j) The uses section about "Assigning vocabularies to content types"... This is very specific to Node entities and still uses the phrase "assign it to a content type", which again is not accurate. And we shouldn't describe the entire field UI interface -- like other fields, we should reference the Field UI help. Look at other field modules for the standard phrases about entities and fields that we've been using.

k) All over the help, actually, the phrasing is very specific to content types, but I'm pretty sure we can add taxonomy fields to any entity now, right?

mparker17’s picture

Assigned: Unassigned » mparker17

I'm working on re-writing this documentation based on the feedback in #21.

mparker17’s picture

Assigned: mparker17 » Unassigned
FileSize
10.16 KB
10.11 KB

I've tried to address the comments in #21 in this patch. However, in doing so, I pretty much had to re-write it. :S


To avoid being specific to node entities, I've followed the policy of referring to bundles as "entity sub-types" and instances of bundles as "entity items" as outlined in #2030569-23: [policy] Decide how to refer to "entities" and "bundles" in D8 UI.

mparker17’s picture

Status: Needs work » Needs review

Whoops, Issue Status.

jhodgdon’s picture

Status: Needs review » Needs work

Yeah, the basic gist of #21 was "this help needs to be completely rewritten". :)

Anyway... this looks great!!! I really like it a lot.

A few minor things to fix:

a) I believe the displayed name of the field is "Term reference", not "Term Reference", but I haven't checked this lately so maybe that has changed?

b) I don't think I would call a Select list a "drop-down menu"... maybe "drop-down list"? It's confusing (IMO) to call it a menu.

c) When "which" is used as a conjunction, it needs to have a comma before it (otherwise it should be "that"). So:
... term linked to a page which lists all entity ...
which -> that

d) I am actually not certain that the term page lists all entity items of all types that have been classified with the term. In the past, it only listed nodes... might want to check the accuracy of that?

mparker17’s picture

I'm short on time today, so I won't be able to write a new patch until at least tomorrow, but here are some responses to points "a" and "b"

a) It is actually Term Reference

b) Sorry, I wanted to distinguish between a "combo box" (a one-line control where you click a button and see the other options) and a "select box" (multi-line control which shows a bunch of options you can command-click to select more than one), but AFAIK the term "combo box" isn't used much outside the Windows world. So I copied the description at https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h... . If anyone has a suggestion for a better term, I would appreciate it.

mparker17’s picture

FileSize
10.15 KB
2.82 KB

Okay, here's a patch. Feedback welcome!

Some additional responses to #25:

b) I looked up the two controls on Wikipedia: they're apparently called Drop-down lists and List boxes respectively, so I'm using those terms.

c) Changed to "that".

d) You are correct, it only lists nodes tagged with that term. Following the terminology used in the hook_help text for the node module, I've changed "entity items" to "node items".

mparker17’s picture

Status: Needs work » Needs review

Whoops, Issue Status.

jhodgdon’s picture

Status: Needs review » Needs work

Whew! This is the last issue I need to review to catch up with the DrupalCon madness! Lots of sprinting took place...

Anyway... Looking good!

On another hook_help issue recently, a request was made to link permission names to the Permissions page (using an anchor like #module-taxonomy to jump to the Taxonomy module section of that page). That would probably be useful here, or you could link the word "permissions" to the Permissions page rather than doing each permission separately.

I also think when we start talking about fields and entities, we need a link to the Entity module help, like I think we have on other field-providing modules... I think the text we're using is something like "see the Entity module help for more information on entities and sub-types..." or ... well, something like that. The wording here just jumps into talking about entity sub-types and doesn't provide any explanation.

And one typo:
... displays nothing when the entity item is displayed as HTML; but displays an RSS category ...
that ; should be a ,

And... You said in your comment that you were going to talk about "node items", but:
...the name of the term linked to a page that lists all content items that have been classified with that term....
I don't think that made it into the patch? Probably it should say "... all Node module content items..." (with a link to the Node module help) anyway? I don't think we can just use terminology from the Node module help without at least a little bit of explanation or a link, because "content items" is too broad, and "node items" is not clear to anyone but old-school Drupal programmers who know what "nodes" are.

jhodgdon’s picture

On #2334011: Document that taxonomy/term is just aware of nodes, a request was made to stress that the taxonomy term listing pages at taxonomy/term/(ID) only show Node content, not all entities classified with a given taxonomy term.

batigolix’s picture

Issue tags: +Documentation, +sprint
greggmarshall’s picture

Issue tags: +Needs reroll

I tried, but got stuck and didn't have time to figure out where I went wrong.

rpayanm’s picture

Status: Needs work » Needs review
FileSize
9.64 KB

Here the reroll.

jhodgdon’s picture

Status: Needs review » Needs work
Issue tags: -Needs reroll

Thanks for the reroll. Patch still needs updates.

jhodgdon’s picture

Instead of having one critical parent issue I have been asked to change the status of each child issue.

This one doesn't feel Major or Critical to me, so leaving as Normal. We should still try to finish it.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
9.68 KB

In the interest of getting the last few hook_help issues done... here's a new patch. I changed pretty much every line of the help, at least a little, from the last patch. So I did not make an interdiff file.

A few notes:
- I took out a stray block of documentation just above the hook_help() function. Not sure why that was there but it wasn't really doing any good and was outdated documentation.
- When linking to another module's help page, you need to check and see if that module is enabled. However, the Node and Field modules are required by Taxonomy, so you don't have to check those, only Field UI.
- I tried to pare down the text to the bare minimum needed to be clear and complete. See what you think...

ifrik’s picture

I filed an issue because term reference fields can be added as term and entity reference - and both have different bugs in Views.
I'll continue reviewing this one tomorrow.

Vijayac’s picture

Issue tags: +SprintWeekend2015

working on this issue

Vijayac’s picture

this patch is still applies cleanly

ifrik’s picture

I'm still working on this.
References can also be added as as Entity Reference field.

ifrik’s picture

I'm still adding the additional options provided by using Entity reference instead of Term reference.

ifrik’s picture

As far as I see the Reference Term field is supposed to be removed in favour of the general Entity Reference field #1847596: Remove Taxonomy term reference field in favor of Entity reference.
However, I've come across a number of issues with regard to sorting and autocomplete: #2412553: Taxonomy terms in an Entity Reference field are not sorted , #2412557: Difference between "Autocomplete" and "Autocomplete (tags style)" is not entirely clear for site builders and #2412569: Allow setting the auto-create bundle on entity reference fields with multiple target bundles so the Use section about Entity Reference will need to wait for a bit longer.

jhodgdon’s picture

I think that the Taxonomy module help should just, for now, document the existence of the Taxonomy field. If it is removed, then the issue that removes it *should* update the hook_help(). I will add a note to that issue.

ifrik’s picture

ifrik’s picture

At the moment there is a duplication of functionality between the Term and the Entity Reference field, as well as some bugs with the Entity Reference field, such as the sorting of terms. I don't know what of that will be fixed or simply be removed.
The patch describes the current state, with the assumption that the bugs get fixed. But in any case the help text need to be checked again once the related issues are dealt with.

I'm sorry, but there is no interdiff because the route to the Taxonomy administration path got changed and I messed up my branches while dealing with that.

jhodgdon’s picture

Status: Needs review » Needs work

I mostly like the latest patch!

A few things I would change:

a) Code indentation problems on the "For more information" line in About, and the entity reference field bullet point in Uses.

b) I'm not sure about the order of the items in Uses. The new topic on adding terms during content creation is talking about fields, before the fields are discussed. I think it either needs to have a "see below" note added, or the section needs to come after the field topics. Or maybe it doesn't need to be a separate topic at all? There could just be a note in the Managing terms topic saying something like "Users can also add terms during content creation, depending on widgets, see ...".

c) In the bullet point about entity reference fields:
Taxonomy terms can be sorted on the by a number of criteria such as name, term ID, vocabulary, term parent and more.
This is garbled. "on the by". I think it should be "Taxonomy terms can be sorted using criteria such as...", and I would get rid of "and more". Also is this the widget sorting the terms or the formatter? Not clear from this sentence. Maybe just leave this out here and add it to the settings/configuration section below? Probably more appropriate there.

d)

'The reference fields have several formatters and widgets available on the Manage display and Manage form display pages, respectively:  '

Extra spaces after : and also that t() does not need the array('!field_ui' => $field_ui_url, '!field' => \Drupal::url('help.page', array('name' => 'field') part.

e)

existing terms of the vocabulary  as check boxes

Extra space here.

f)

The <em>Autocomplete</em> widget of an <em>Entity Reference</em> field, displays text fields

Delete the comma

g)

The <em>Autocomplete</em> widget of an <em>Entity Reference</em> field, displays text fields in which users can type terms based on the <em>Allowed number of values</em>.

I do not understand how this is related to Allowed number of values?

h) Are all the formatters available for both Term Reference and Entity Reference fields?

i) This section at the end:

+      $output .= '<dt>' . t('Classifying entity items') . '</dt>';
+      $output .= '<dd>' . t("If a Term Reference field has been set up for an entity, and a widget assigned on the create or edit form, any user with permission to create or edit entity items can categorize them on the create or edit form.") . '</dd>';

Maybe this needs editing or should just be removed now?

chintan4u’s picture

Status: Needs work » Needs review
FileSize
11.4 KB

@jhodgdon I have fixed points a) c) d) e) f) in below path, remaining are still there.

Status: Needs review » Needs work

The last submitted patch, 48: 2091367-taxonomy-help-48.patch, failed testing.

chintan4u’s picture

Status: Needs work » Needs review
FileSize
11.4 KB

Correcting php syntax error.

rteijeiro’s picture

Status: Needs review » Needs work
+++ b/core/modules/taxonomy/taxonomy.module
@@ -50,30 +50,43 @@
+      $output .= '<p>' . t('For more information, see the <a href="!taxonomy">online documentation for the Taxonomy module</a>.', array('!taxonomy' => 'https://drupal.org/documentation/modules/taxonomy/')) . '</p>'; ¶

There is a trailing whitespace in this line. Sorry for the nitpick :P

chintan4u’s picture

Status: Needs work » Needs review
FileSize
11.4 KB
rteijeiro’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
338.86 KB

Great! Now it's perfect!

Looks RTBC for me!

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for the patches and screen shot! This looks pretty good, but it's not quite done, sorry:

a) Before any UL list, the preceding line should end in a :. The first list doesn't satisfy this general documentation standard.

b) I do not understand this part at all:

In addition to the default <em>Reference method</em>, you can create a view with an <em>Entity Reference</em> display, using the <em>Views module</em> if enabled. Taxonomy terms can be sorted using criteria such as name, term ID, vocabulary, term parent.

Really I think we should remove it. This is talking about Views, and it is I think not totally relevant to that section. If we want to talk about Views it should be a separate DT/DT section.

c) I'm really not sure we still need that last "Classifying entity content" item in the help... it seems redundant to the "Adding fields" item, and has less information. Maybe we should just change the heading of the adding fields item so that it is "Classifying entity content" and the first sentence says something like:

A user with the Administer fields permission for a certain entity type may add reference fields to the entity, which will allow entities to be classified using taxonomy terms.

Shreya Shetty’s picture

Status: Needs work » Needs review
FileSize
10.9 KB

Hi jhodgdon ,
I have made changes according to your suggestion.Please find the patch.

jhodgdon’s picture

Status: Needs review » Needs work

Interdiff files are always requested when you make a new patch: https://drupal.org/documentation/git/interdiff -- Next time please! :)

So, this looks much better, thanks!

I read through the patch file carefully and I am only seeing these very small problems:

a) in Classifying section:

may add reference fields to the entity,which will allow entities to be classified

needs space after comma

b) I'm not really comfortable with how the : before the UL turned out. Really a list needs to be prefaced by a short phrase saying what's in the list, and what it says now is:

See the ... help pages for general information on fields and how to create and manage them:

That doesn't make sense. We should probably end that sentence with a . and then move the "two types of fields" sentence from earlier in that paragraph to the end, and end that with a :, because that is what the bullet list is about. Make sense?

c) Do you think it would make more sense to move the "Adding new terms during content creation" section after the "Classifying content" section, since that section tells about the two field types referenced in "Adding new terms"?

d) In the Configuring displays topic:

 t('The reference fields have several formatters and widgets available on the Manage display and Manage form display pages, respectively:', array('!field_ui' => $field_ui_url, '!field' => \Drupal::url('help.page', array('name' => 'field')))) .

The two !things in the array (!field_ui and !field) are not actually used in the t() text and should be removed. Actually... There are several other places in the patch where that is the case, such as in Managing Vocabularies. Please go through the patch and make sure everything in the array() arguments to t() is actually used, and remove pieces that aren't.

Anyway... very close, and thanks again!

chintan4u’s picture

Hi jhodgdon

How about following text ? Or Would you like to move entire ("A user with the Administer fields " )sentence below?

Classifying entity content
A user with the Administer fields permission for a certain entity type may add reference fields to the entity, which will allow entities to be classified using taxonomy terms. See the Field module help and the Field UI help pages for general information on fields and how to create and manage them. This can either be a Term reference or an Entity reference field:

chintan4u’s picture

Status: Needs work » Needs review

Hi jhodgdon

Points a) c) d) covered let me know your thoughts on point b)....Accordingly I will upload the updated patch.

Shreya Shetty’s picture

Hi jhodgdon,
I have changed as per your suggestions and removed array which wasn't necessary in t(). So please find the updated patch and tell if further any improvements should be made.
Thanks.

Shreya Shetty’s picture

FileSize
10.74 KB
Shreya Shetty’s picture

Hi jhodgdon ,
Please find the updated patch.

Shreya Shetty’s picture

Shreya Shetty’s picture

chintan4u’s picture

@Shreya Shetty

--If a person is working on issue and adding patches or communicating with reviewers then extend some courtesy before creating own patch (with wrong patch names)
-- Interdiff files are different files than patch files.

jhodgdon’s picture

@chintan4u and @Shreya Shetty -- Both of you have supplied patches in the last few days and communicating, much appreciated! But yes, it is better to have one person working at a time on an issue. Normally, to signify that you want to be the one person who is working on an issue, and plan to come back and make new patches, etc., you should set the "Assigned" field on the issue to yourself.

Neither of you did this, so 22 hours ago @Shreya Shetty made a patch, which I reviewed, and then 12 hours ago @chintan4u made a comment, 11 hours ago @Shreya Shetty made a few more patches. To me it seems like @Shreya Shetty was working on the issue when @chintan4u stepped in, so please @chintan4u do not get angry at @Shreya Shetty? Thanks!

@chintan4u, maybe for now you can help on this issue by reviewing patches and making suggestions? Thanks!

I'll review the patch in a separate comment...

jhodgdon’s picture

Or alternatively, maybe @Shreya Shetty can step aside and volunteer to review patches... Anyway let's try to be civil, and whoever wants to continue with this issue should assign it! :)

Shreya Shetty’s picture

Assigned: Unassigned » Shreya Shetty

Hi jhodgdon,
Thanks for appreciating our efforts I have assigned this issue to myself.

jhodgdon’s picture

Status: Needs review » Needs work

Regarding the latest patches and files...
@Shreya Shetty - please read https://drupal.org/documentation/git/interdiff -- your interdiffs are not right.

Anyway, that aside, looking at the latest patch in #61, I think it is very close.

a) Classifying topic:

A user with the <em>Administer fields</em> permission for a certain entity type may add reference fields to the entity, which will allow entities to be classified using taxonomy terms. See the <a href="!field">Field module help</a> and the <a href="!field_ui">Field UI help</a> pages for general information on fields and how to create and manage them. This can either be a <em>Term reference</em> or an <em>Entity reference</em> field:

Minor suggestion: In the last sentence it says "This". Given that what "this" references is two sentences back, I think maybe it should say "The reference field can either..." instead?

b) Same place: I also think that the UL should be inside the same DD entry, so don't close the DD and open a new one before the UL. In other words, take out the /DD and the DD here:

. '</dd>';
+      $output .= '<dd>';
+      $output .= '<ul>';

c) Same as (b) in the "Configuring displays" section.

I think if these three small things are fixed, we'll be good to go!

Shreya Shetty’s picture

Status: Needs work » Needs review
FileSize
4.14 KB

Hey jhodgdon,
Thanks for the review and suggestions.I have made the changes accordingly.

Status: Needs review » Needs work

The last submitted patch, 69: 2091367-hook-help-taxonomy-patch-69.patch, failed testing.

Shreya Shetty’s picture

Shreya Shetty’s picture

Status: Needs work » Needs review
lokapujya’s picture

Missing an interdiff.

The last submitted patch, 69: 2091367-hook-help-taxonomy-patch-69.patch, failed testing.

jhodgdon’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Yes -- in the future, PLEASE learn how to make an interdiff, and include one every time you make an update to an issue that had a patch previously.

Anyway, I've reviewed the latest patch and I think it's great!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 62afca1 and pushed to 8.0.x. Thanks!

  • alexpott committed 62afca1 on 8.0.x
    Issue #2091367 by Shreya Shetty, mparker17, Sree, chintan4u, jhodgdon,...

Status: Fixed » Closed (fixed)

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