feat: HTML Table data keys synchronize order with Heading keys - #7409
Conversation
michalsn
left a comment
There was a problem hiding this comment.
This is a helpful feature. Cool!!!
I have some doubts about the variable/method names you chose - a little too long for my taste. But you don't have to change them right away. I'm looking for others' opinions on this.
kenjis
left a comment
There was a problem hiding this comment.
Thank you. This is a great enhancement!
Please add this new feature to the changelog
https://github.com/codeigniter4/CodeIgniter4/blob/4.4/user_guide_src/source/changelogs/v4.4.0.rst#others-1
and link to the description page so that more users will know about it.
See how to link: https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/documentation.rst#to-a-section
| /** | ||
| * Order each inserted row by heading keys | ||
| */ | ||
| public bool $syncRowsWithHeading = false; |
There was a problem hiding this comment.
Is there a need to make this public?
There was a problem hiding this comment.
All other properties are public, I will keep it for consistency
There was a problem hiding this comment.
All other properties seems to be wrong, but changing them is breaking changes...
There was a problem hiding this comment.
publicfor current consistency- change only this property to
protectedorprivate
There was a problem hiding this comment.
I would like this new property to be private to enforce API encapsulation. We can increase visibility later if needed.
For the others, we can retain them as public to prevent breaking changes.
| See :ref:`controller-default-method-fallback` for details. | ||
| - **Filters:** Now you can use Filter Arguments with :ref:`$filters property <filters-filters-filter-arguments>`. | ||
| - **Request:** Added ``IncomingRequest::setValidLocales()`` method to set valid locales. | ||
| - **Table:** Addedd ``Table::setSyncRowsWithHeading()`` method to synchronize row columns with heading. See :ref:`table-sync-rows-with-headings` for details. |
There was a problem hiding this comment.
| - **Table:** Addedd ``Table::setSyncRowsWithHeading()`` method to synchronize row columns with heading. See :ref:`table-sync-rows-with-headings` for details. | |
| - **Table:** Added ``Table::setSyncRowsWithHeading()`` method to synchronize row columns with headings. See :ref:`table-sync-rows-with-headings` for details. |
kenjis
left a comment
There was a problem hiding this comment.
Thank you for updating.
I commented two minor things inline.
|
@rumpfc Thank you! |
Description
Tableis an ideal builder class to turn data and query results into an HTML table. That safes a lot of time to write an own builder.However there is a weakness when using custom Heading and data (no query results): If the number of headings does not match the number of columns in each row, the table becomes unbalanced. Furthermore when fetching data from APIs or other data sources (i.e. in XML or JSON), data must be manually rearranged to match the correct positions.
This feature introduces method
Table::setSyncRowKeysWithHeadingKeysto internally rearrange the positions to match a certain key defined in setHeading if used with an associative array. Data row keys not defined inheadingare filtered out, and missing keys in data rows (according toheading) will produce an empty cell.This produces HTML table
Checklist: