Problem/Motivation
Since we have already #80855: Add element #type table and merge tableselect/tabledrag into it and we are #1876712: [meta] Convert all tables in core to new #type 'table' and after that will #1876714: Remove #type 'tableselect', we need to build out our table rendering tests.
Proposed resolution
Build on our tests in Drupal\system\Tests\Theme\TableTest to include tableselect/tabledrag coverage in tests for #type table.
Remaining tasks
See Proposed Resolution.
User interface changes
None.
API changes
None.
Related Issues
Several of http://drupal.org/project/issues/search?text=table&projects=&status[]=Open, specifically:
#1876712: [meta] Convert all tables in core to new #type 'table'
#1876714: Remove #type 'tableselect'
#1876718: Allow modules to inject columns into tables more easily
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | interdiff.txt | 1.46 KB | nitesh pawar |
| #16 | expand_render_test-1975152-16.patch | 2.6 KB | nitesh pawar |
| #12 | expand_render_test-1975152-12.patch | 2.66 KB | nitesh pawar |
| #9 | expand_render_test-1975152-9.patch | 2.63 KB | joelpittet |
| #6 | drupal-type-table-render-tests--1975152-5.patch | 2.86 KB | steveoliver |
Comments
Comment #1
steveoliver commentedtitle, tag
Comment #2
andypostCore 8 introduced some new elements and it would be good to collect em in Drupal\system\Tests\Common\RenderTest
Comment #3
webchickHm. I think given there are at least 3-4 issues that are changing this stuff and punting on test coverage, we should escalate this a bit.
Comment #4
steveoliver commentedComment #5
steveoliver commentedUpdate: I'm making progress here. I haven't forgotten, just lagging.
Comment #6
steveoliver commentedNot much here so far, but I gotta unassign to get back to Twig review/profiling...
Comment #7
jhedstromChanging status since there is a start here.
Comment #8
joelpittetxpath may be better to avoid having to keep track of whitespace changes?
Probably can be 'table templates'?
Comment #9
joelpittetRe-roll and see what test bot thinks.
Comment #12
nitesh pawar commentedPatch on new revision
Comment #15
tim.plunkettgetInfo is obsolete, you need a @group annotation on the test class
These should be public
This can't be empty
Comment #16
nitesh pawar commentedThanks @tim.plunkett changes done as you suggested.
Comment #18
joelpittetThis may be ripe for using $this->removeWhiteSpace(); Just a guess. And maybe render_render() can be just Drupal::service('renderer')->renderPlain() to avoid testing asset attachment stuff.
Comment #32
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #33
smustgrave commentedBelieve this may still be relevant as the related issues are still in NW.