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 CreditAttribution: 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 CreditAttribution: steveoliver commentedComment #5
steveoliver CreditAttribution: steveoliver commentedUpdate: I'm making progress here. I haven't forgotten, just lagging.
Comment #6
steveoliver CreditAttribution: 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 CreditAttribution: Nitesh Pawar at Trigyn Technologies Ltd 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 CreditAttribution: Nitesh Pawar at Trigyn Technologies Ltd 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.