Problem/Motivation
API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...
The documentation is missing a lot of details. Table seems to support #footer and #caption, which are not listed.
Second, in the example, the table rows are not being added to the #rows element. This could be confusing.
Steps to reproduce
See documentation of in table class in core/lib/Drupal/Core/Render/Element/Table.php
Proposed resolution
Add an example using the #rows element
Add comments to the existing example (without the #rows element) to make it less confusing
Add information about #footer and #caption
Remaining tasks
a. Fix issue summary
b. Review
c. Commit
User interface changes
NA
API changes
NA
Data model changes
NA
Release notes snippet
NA
Issue fork drupal-3071143
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
jasonsafro commentedFYI - The same issues exist on all 8.x versions of the documentation. Since google usually returns the 8.2 version, it would be helpful to update all versions of the 8.x docs.
Comment #9
larowlanComment #10
larowlanComment #11
smustgrave commentedComment #12
chrisdarke commentedMigrating Pittsburgh 2023 to Pittsburgh2023 tag for cleanup
Comment #13
danielmme commentedI will work on this in Drupal Con
Comment #25
karimb commentedActually the example is working fine. No need to explicitly declare the #rows property when adding sub-elements (or children) to $form['contacts'] (the main table element). When adding children like $form['contacts'][$i][...] these children will populate the #rows property automatically in preRenderTable(). I added some comments for better understanding.
I also added a new example with the #rows property explicitly declared and added information for #footer and #caption.
(I made a merge request to 11.x)
Hope this help ;)
Comment #26
karimb commentedComment #27
smustgrave commentedThank you for working on this @KarimB. Since it seems the original issue summary is different from what you found could you update the issue summary please
Comment #28
anushrikumari commentedRemoving the Novice tag as this issue has been to various contribution events, so keeping this in Novice won't be helpful.
Comment #29
karimb commentedUpdate the issue summary now that we sure the original example is working fine.
Comment #30
karimb commentedComment #31
smustgrave commentedAdded the issue template for guidance, also title doesn't seem to accurately describe the issue. Sorry I should have flagged that earlier in #27, that's my fault.
Comment #32
karimb commentedComment #33
karimb commentedComment #34
karimb commentedComment #35
karimb commentedComment #36
karimb commentedComment #37
smustgrave commentedThanks, this reads well to me so lets see!
Comment #38
quietone commentedThanks for contributing to better documentation!
I'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions or other work to do.
Thanks to @smustgrave for adding the standard template. That is a big help!
I applied the diff locally and read the changes. What I read is a great improvement! There are lines that are not wrapped correctly, so those will need to be changed. And I have a question which I left in the MR.
Also want to say that this is staying within scope by not changing the 9 usages of the old array syntax in the existing example. It does mean the example is a mixture of old and new array syntax which is unfortunate and another committer my disagree and want that changed here. But there are coding standard issues to fix the old array syntax in @code @endcode examples.
Comment #39
samsylve commentedI am working on this issue now as part of a novice contribution workshop at Symetris.
Comment #40
samsylve commentedFixed wrapping on docblocks.
Fixed array format.
Hope this helps.
Comment #41
smustgrave commentedSeems wrapping feedback has been addressed.
Comment #42
larowlanIssue credits
Comment #45
larowlanChecked out locally and verified all the text wrapped at 80 chars.
Committed to 11.x and backported to 10.2.x
Thanks folks 💪