API page: https://api.drupal.org/api/drupal/core%21includes%21theme.inc/function/t...

There are 3 mentions of HTML tags in this text, all of which different:

    url: (optional) The url object to link to. If omitted, no a tag is printed out.
    attributes: (optional) Attributes for the anchor, or for the <span> tag used in its place if no 'href' is supplied. If element 'class' is included, it must be an array of one or more class names.

If the 'href' element is supplied, the entire link array is passed to l() as its $options parameter.
attributes: A keyed array of attributes for the UL containing the list of links.

- "no a tag" is really confusing, because the 'a' just looks like the word 'a'.
- "the <span> tag" is using angle brackets to mention a tag.
- "A keyed array of attributes for the UL containing" is using no angle brackets, but capitals to mention a tag.

CommentFileSizeAuthor
#12 inconsistent_use_of-2710685-12.patch1.35 KBdimaro
#12 interdiff-2710685-9-12.txt745 bytesdimaro
#9 inconsistent_use_of-2710685-9.patch1.35 KBdimaro
#9 interdiff-2710685-5-9.txt1.36 KBdimaro
#5 D8-inconsistent-use-of-tags-2710685-5.patch1.21 KBpushpinderchauhan
#3 D8-inconsistent-use-of-tags-2710685-3.patch1.29 KBpushpinderchauhan

Comments

joachim created an issue. See original summary.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

I suggest that all of them use UPPER_CASE for the tag names.

pushpinderchauhan’s picture

pushpinderchauhan commented
Status: Active » Needs review
StatusFileSize
new D8-inconsistent-use-of-tags-2710685-3.patch1.29 KB

Please review.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks for the patch! Notes:

  1. +++ b/core/includes/theme.inc
@@ -585,14 +585,14 @@ function template_preprocess_datetime_wrapper(&$variables) {
- *     - url: (optional) The url object to link to. If omitted, no a tag is
+ *     - url: (optional) The url object to link to. If omitted, no anchor tag is

    This is a good way to change this line, because the rest of the text refers to "anchor" as well. 

  2. +++ b/core/includes/theme.inc
@@ -585,14 +585,14 @@ function template_preprocess_datetime_wrapper(&$variables) {
- *     - attributes: (optional) Attributes for the anchor, or for the <span>
+ *     - attributes: (optional) Attributes for the anchor, or for the <SPAN>

    We should either say <span> or SPAN but not <SPAN>. Sorry I was not clear. I guess <span> is probably the best, and the API module seems to handle it OK. 

  3. +++ b/core/includes/theme.inc
@@ -585,14 +585,14 @@ function template_preprocess_datetime_wrapper(&$variables) {
- *   - attributes: A keyed array of attributes for the UL containing the
+ *   - attributes: A keyed array of attributes for the <UL> containing the

    See previous note.

pushpinderchauhan’s picture

pushpinderchauhan commented
Status: Needs work » Needs review
StatusFileSize
new D8-inconsistent-use-of-tags-2710685-5.patch1.21 KB

Thanks jhodgdon! Please take a look again.

joachim’s picture

joachim commented

> I guess is probably the best, and the API module seems to handle it OK.

Should that be added to the documentation standards?

dimaro’s picture

dimaro commented 
+++ b/core/includes/theme.inc
@@ -585,14 +585,14 @@ function template_preprocess_datetime_wrapper(&$variables) {
+ *   - attributes: A keyed array of attributes for the <ul> containing the
  *     list of links.

@joachim Maybe we need use all space in this line?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

RE #6, if you want to add to the coding standards or docs standards, see
https://www.drupal.org/project/coding_standards
There is a process. It actually kind of works now. But we can't just go editing the page.

And yes, we could do a slightly better job of wrapping (see #7).

+++ b/core/includes/theme.inc
@@ -585,14 +585,14 @@ function template_preprocess_datetime_wrapper(&$variables) {
+ *     - url: (optional) The url object to link to. If omitted, no anchor tag is

If we are fixing this line, could we also change

The url object => the (full namespace of the Url class) object

dimaro’s picture

dimaro commented
StatusFileSize
new interdiff-2710685-5-9.txt1.36 KB
new inconsistent_use_of-2710685-9.patch1.35 KB

Apply changes commented above on #7 and #8.

joachim’s picture

joachim commented
Related issues: +#2711751: add a standard for referring to HTML tags in code documentation
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

This looks good, thanks! Except:

+++ b/core/includes/theme.inc
@@ -585,15 +585,15 @@ function template_preprocess_datetime_wrapper(&$variables) {
+ *     - url: (optional) The \Drupal\Core\Link object to link to. If omitted, no

I am pretty sure that this is a Url object, not a Link object?

dimaro’s picture

dimaro commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2710685-9-12.txt745 bytes
new inconsistent_use_of-2710685-12.patch1.35 KB

I don't know what I was thinking!... My apologies.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks! Looks fine to me.

dimaro’s picture

dimaro commented
Issue tags: +DrupalCampES

I have been working in this issue at DrupalCamp Spain.
Tagging this issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed c9750a3 and pushed to 8.1.x and 8.2.x. Thanks!

Status: Fixed » Closed (fixed)

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