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:
- write the hook_help text according to help guidelines

Files: 
CommentFileSizeAuthor
#16 Menu_161.png20.18 KBbatigolix
#14 Screen Shot 2014-02-06 at 23.51.47.png81.5 KBvijaycs85
#9 2091397-Create-hook_help-for-Datetime-module-9.patch2.32 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 59,915 pass(es).
[ View ]
#9 interdiff.txt1.84 KBbatigolix
#7 interdiff.txt3.43 KBbatigolix
#7 2091397-Create-hook_help-for-Datetime-module-7.patch2.33 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 59,833 pass(es).
[ View ]
#5 interdiff.txt0 bytesbatigolix
#5 2091397-Create-hook_help-for-Datetime-module-5.patch2.18 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 60,206 pass(es).
[ View ]
#3 2091397-Create-hook_help-for-Datetime-module-3.patch2.22 KBtargoo
PASSED: [[SimpleTest]]: [MySQL] 58,264 pass(es).
[ View ]
#1 2091397-Create-hook_help-for-Datetime-module-1.patch2.05 KBtargoo
PASSED: [[SimpleTest]]: [MySQL] 58,759 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,759 pass(es).
[ View ]

Hey

Here the patch.

Status:Needs review» Needs work

Thanks for the patch! It's a great start. I have a few comments:

a) In reading the first About sentence... The fact that the module provides a form element is only of interest to programmers, and the vast majority of the readers of help will not be programmers (nor will the vast majority of programmers read the help). So I think we need to make clear to the non-programmers what that means. Maybe the first sentence should talk about the field, and then there could be a second sentence saying something like "... also provides a Form API element called 'datetime' for use in programming modules'" or something like that.

b) Similarly, we want to refer to the field using the label that non-programmers see in the user interface, which is "Date". This applies throughout the help text. Also the formatters should be documented using their displayed names, which are "Plain" and "Default".

c) I'm also uncomfortable with saying it's a "idealized naive date"... What exactly is naive about it? How about just saying that the field can store either dates or dates and times -- people will understand that?

d) URLs need to be reformatted. See https://drupal.org/node/632280#url-note

e) Probably the Validating heading in Uses should end in "values" not "value"?

Status:Needs work» Needs review
StatusFileSize
new2.22 KB
PASSED: [[SimpleTest]]: [MySQL] 58,264 pass(es).
[ View ]

Hello

Many thanks for the support.

Here is a new patch following your advices.

Status:Needs review» Needs work

Thanks! Looks better! A few things still to fix:

a) I forgot this in the previous review, but the link to the online documentation is not following our template:
https://drupal.org/node/632280

b) From #2:

b) Similarly, we want to refer to the field using the label that non-programmers see in the user interface, which is "Date". This applies throughout the help text. Also the formatters should be documented using their displayed names, which are "Plain" and "Default"."

This is still a problem. The first sentence in About refers to a "datetime" field, for instance -- it's not called "datetime" in the user interface. That needs to say "Date" instead. And probably in that same sentence, it should say it can store both dates and times. Then you can get rid of the last sentence in About. Similar changes need to be made in the rest of the text.

c) This sentence needs some grammatical editing:
The plain text which uses the ISO format and the defaut which support built-in human readable formats.
It's not a sentence -- remove the words "which" and it would be. Also "default" is misspelled, and "support" should be "supports". And if you are going to say "the", you need to say "The plain text format" not just "the plain text". Same for default. And human-readable needs to be hyphenated.

d) There is too much "built-in" and "by default". Just tell what the module does.

Thanks!

Issue summary:View changes
Status:Needs work» Needs review
Parent issue:» #1908570: [meta] Update or create hook_help() texts for D8 core modules
StatusFileSize
new2.18 KB
PASSED: [[SimpleTest]]: [MySQL] 60,206 pass(es).
[ View ]
new0 bytes

Patch addresses points raised in #5

I am still in doubt about the points Displaying dates and Validating dates in the Uses section. They do not contain very much information

Component:documentation» datetime.module
Priority:Normal» Critical
Status:Needs review» Needs work

Datetime module has no hook_help currently, so this is a Critical issue (violates Core Gates). Moving to that component so we can get the maintainers to approve the text.

Other review thoughts:

a)

The Datetime module allows you to create fields for storing dates and time.

Should probably be "dates and times"?

b)
In About, the link to "Field module help" has the word "module" in it, but "Field UI help" doesn't. Let's be consistent. (Oh no, is that just copy/pasted from all the other field module help pages?!?)

c) In About... it looks like this module creates both a "datetime" and "datelist" element, and it would also be more correct to say that they are for the "Form API" not "form API".

d) In the UI, the field type is "Date", not "date", so it should be referred to that way in the help. It might be useful to mention this in the About as well, because it says the field is for dates and times, so it might not be obvious that a field called "Date" is used for that.

e) Under Displaying, "fotmatter" is misspelled.

f) I don't think we need the "Validating" item under Uses. I think "Displaying" is fine, but maybe we should explain what the ISO format is or provide a link? And I wouldn't say that the ISO format is not "human readable" really. I find it quite easy to read myself. :) Maybe we can say something like that the "Default" choice lets you choose which format to use, and also provide a link to the date settings page where you can define the formats? I'm assuming it lets you choose from the date formats you have defined for your system...

g) Under Displaying... Before "which" you need to have a comma, and "human-readable" should be hyphenated, if we leave that in there.

Status:Needs work» Needs review
StatusFileSize
new2.33 KB
PASSED: [[SimpleTest]]: [MySQL] 59,833 pass(es).
[ View ]
new3.43 KB

Patches addresses points in #6:

a) done
b) done & oh yes, it is like that in the "template text" for field type modules
c) done
d) done
e) done
f) done
g) these were removed from the text

Status:Needs review» Needs work
Issue tags:+Needs manual testing

Maybe in About "allows storing" should be "stores" in "The Datetime module provides a Date field that allows storing dates and times."?

And then in that 2nd About sentence, you list two Form API elements, so it should say "It also provides the Form API elements datetime and datelist..." not "element".

Everything else looks great! This will need a manual review to makes sure the links work and formatting is OK.

Status:Needs work» Needs review
StatusFileSize
new1.84 KB
new2.32 KB
PASSED: [[SimpleTest]]: [MySQL] 59,915 pass(es).
[ View ]

Points mentioned in #8 fixed

The manual review (#8) still needs to be done

Looks great to me! Now ready for a manual test:
- Verify that all the links work
- Verify that all mentions of pages/text within the UI match what is seen in the UI
- Verify that the formatting is OK.

I'm not sure who the maintainers of this module are -- it's not in MAINTAINERS.txt that I can see -- but if someone is maintaining it and could review the patch, that would also be good.

Issue tags:+Novice

I gave this a manual test and it looks good.

One thing that confused me though and I don't know the answer. When choosing the formatting options for a Date field on Manage Fields, I have a lot more options available besides the 3 that are on the Date and Time config page. This is not mentioned in the help and I have no idea what they are.

Do you know where they come from? Should we mention them in the help?

Status:Needs review» Reviewed & tested by the community

@jhodgdon not sure which fields you are referring. I can see 'Allowed number of values' with select and text field which is default option for all fields and doesn't need any documentation update.

RTBC as the help text looks good to me as well.

StatusFileSize
new81.5 KB

Adding screenshot for #13

Status:Reviewed & tested by the community» Needs work

Sorry for not being clear! In #12, I was talking about the *formatting* options (Manage Display). The help in the latest patch says:

If you choose the Default formatter, you can choose a format from a predefined list that can be managed on the Date and time formats page.

However, when I go to the date and time formats page, I see only 3 options, but on Manage Display, I have more options. I don't know where these are coming from, and the help is not accurate as it is.

StatusFileSize
new20.18 KB

erm, yeah, you mean that long list

date formats

i'll have a look

Unfortunately I cannot figure this out. They are constructed in the class class DateTimeDefaultFormatter (line 155)

$format_types = $this->dateStorage->loadMultiple();

Too much OO magic for me :(

Yes. I think we need the maintainers of the Datetime module to explain what is going on here and propose help wording to explain it, because I have no idea either.

Unfortunately I am not sure who those maintainers are. :(

Status:Needs work» Postponed

We just had a discussion in IRC about this with Berdir and Gabor...

Apparently the list does come from the same place, but the Manage Display list includes "locked" formats, and the Date and Time Formats admin page does not currently show them. Plus, the names are not displayed the same way on the two pages.

I have filed an issue to address that:
#2191189: UI disconnect between Date Time Formats admin page and Manage Display for the field
and am postponing this issue until it is addressed, because I don't think we can write coherent help until that is decided and the UI may be changing.

It is loading all config entities of type 'date_format' (type defined in /core/modules/system/lib/Drupal/system/Entity/DateFormat.php). The config entities of this type(YML file with prefix 'system.date_format') can be defined from any module. For core, we got just system module(e.g. /core/modules/system/config/system.date_format.html_date.yml) implementing all the formats listed.

Status:Postponed» Needs work

This should not be blocked on #2119903: Adding a date format that is locked (and currently *not* shown in the list of existing formats) returns error "already in use". huh?!. We should add a hook_help() that explains how things work, and close this *critical*.

Then, if we choose to fix the other issue, we can update the help text.

I don't really see how we can write sensible help text until the UI is behaving in a sensible manner that can be explained to mere non-programming mortals, but OK.

Assigned:Unassigned» adevms

Assigned:adevms» Unassigned