Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is a regression from: #1939008: Convert theme_table() to Twig.
Follow-ups from:
#806982: Tables should take an optional footer variable and produce <tfoot>
#2282683: Responsive tables do not have tests
Problem/Motivation
- Classes are only added based on header indices, which match the row indices. This causes headers, which are associative, to not propagate the responsive classes to the rows, which are indexed.
- Example of
/admin/content/comment
:
Proposed resolution
- Use a separate counter (like it was before) and not the header index.
Remaining tasks
- Create patch.
User interface changes
None.
API changes
None
Comment | File | Size | Author |
---|---|---|---|
#1 | regression_responsive-2296859-1-PASS.patch | 3.74 KB | markhalliwell |
#1 | regression_responsive-2296859-1-FAIL.patch | 1.21 KB | markhalliwell |
Comments
Comment #1
markhalliwellComment #2
markhalliwellHere's the screenshot of it fixed:
Comment #4
markhalliwellComment #5
sunHm. To me, this is a matter of architectural expectations.
My natural DX expectation would be
theme_table()
blows up.In other words, my DX expectation would rather be the "opposite" of this patch:
IMO, fixing the error/exception handling for the root cause (inconsistent column keys) would deliver a much better DX + reliability. Especially for alter hook operations that are adding columns/data to a certain table.
Thoughts?
Comment #6
markhalliwellThat would indeed be an improvement (one I would gladly follow/support). However this issue is about fixing an obvious currently broken HEAD caused by the conversion of table theme functions to twig. I do not wish to drag this particular issue out. Please open a follow-up.
Comment #7
sunComment #10
star-szrComment #11
alexpottCommitted 5fe283c and pushed to 8.x. Thanks!