This issue has novice tasks. If you are an experienced core developer and have multiple commit mentions, please review novices' work on these tasks rather than doing them yourself. Feedback from experienced contributors is valued.

Problem/Motivation

#1939062: Convert theme_item_list() to Twig added item-list.html.twig but the variables documented in the Twig template docblock are not 100% accurate. Mainly the 'items' component is not documented accurately:

  1. Each item in the item array no longer has a #wrapper_attributes property.
  2. Each item contains two subvalues: 'value' and 'attributes'.

Proposed resolution

Fix the documentation.

Remaining tasks

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#10 d8-updated_doc-2226853-10.patch849 bytesacouch
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,578 pass(es). View
#7 d8-updated_doc-2226853-7.patch843 bytesfilijonka
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,883 pass(es). View
#4 d8-updated_doc-2226853-4.patch867 bytesfilijonka
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es). View
#1 d8-updated_doc-2226853.patch818 bytesfilijonka
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,687 pass(es), 72 fail(s), and 15 exception(s). View

Comments

filijonka’s picture

Assigned: Unassigned » filijonka
Status: Active » Needs review
FileSize
818 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,687 pass(es), 72 fail(s), and 15 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 1: d8-updated_doc-2226853.patch, failed testing.

Cottser’s picture

+++ b/core/modules/system/templates/item-list.html.twig
@@ -4,9 +4,8 @@
- * - items: A list of renderable items. Each item has a #wrapper_attributes
- *   property which contains any HTML attributes which should be applied to the
- *   <li> tag.
+ * - items: A list of renderable items. Each item holds two subvalues attributes and value
+ *   which is applied within the <li> tag.

Thanks @filijonka. A couple things:

I think we should split out the items and make a sublist to show more obviously that items contains 'attributes' and 'value'. Check node-add-list.html.twig for a somewhat similar example.

The first line of the comment goes over 80 characters, see https://drupal.org/node/1354#drupal.

filijonka’s picture

FileSize
867 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es). View

thanks for the feedback @cottser

filijonka’s picture

Status: Needs work » Needs review
Cottser’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/templates/item-list.html.twig
@@ -4,9 +4,9 @@
+ * - items: A list of renderable items. Each item has following properties
+ *   - attributes: HTML attributes to be applied to the list
+ *   - value: The content of the list element

Thanks, looking better! A few things:

'items' is just an array, not a render array. So we shouldn't say it contains renderable items. I suggest something along these lines:

"A list of items. Each item contains:"

Then for the sub-elements, each of those needs to be a complete sentence (ends in a period) per https://drupal.org/node/1354#general. Other than that looks pretty good to me :)

filijonka’s picture

Status: Needs work » Needs review
FileSize
843 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,883 pass(es). View
Cottser’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/templates/item-list.html.twig
@@ -4,9 +4,9 @@
+ *   - attributes: HTML attributes to be applied to the list.
...
  * - attributes: HTML attributes to be applied to the list.

I don't think we should have the same documentation for these two variables because they are quite different. One is for the ul/ol and one is for the li. I think this is looking good otherwise :)

filijonka’s picture

ehm that was odd, i thought i wrote listelement on the the first one, would that be alright? i.e HTML attributes to be applied to the listelement

acouch’s picture

FileSize
849 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,578 pass(es). View

Attached is updated wording:

+++ b/core/modules/system/templates/item-list.html.twig
@@ -4,9 +4,9 @@
+ *   - attributes: HTML attributes to be applied to each list item.
...
  * - attributes: HTML attributes to be applied to the list.
acouch’s picture

Status: Needs work » Needs review
filijonka’s picture

Status: Needs review » Reviewed & tested by the community

thanks acouch

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 98b2898 and pushed to 8.x. Thanks!

  • Commit 98b2898 on 8.x by alexpott:
    Issue #2226853 by filijonka, acouch: Variables documented in item-list....

Status: Fixed » Closed (fixed)

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

filijonka’s picture

Assigned: filijonka » Unassigned