Big addColumn, addField and Seed refactor (PART3) #225
Conversation
…refactor-columns-fields
romaninsh
changed the title
Codecov Report
@@ Coverage Diff @@
## epic/atk-core-refactor #225 +/- ##
=============================================================
+ Coverage 41.23% 53.23% +11.99%
+ Complexity 1053 1049 -4
=============================================================
Files 58 58
Lines 2488 2502 +14
=============================================================
+ Hits 1026 1332 +306
+ Misses 1462 1170 -292
Continue to review full report at Codecov.
|
…ture/actually-refactor-columns-fields
…ture/cleanup-errors
|
This still needs more work, test-suite must pass. |
…ture/cleanup-errors # Conflicts: # docs/table.rst # src/Table.php
| @@ -141,7 +141,7 @@ public function addQuickSearch($fields = []) | |||
| public function addAction($label, $action) | |||
| { | |||
| if (!$this->actions) { | |||
| $this->actions = $this->table->addColumn('TableColumn/Actions'); | |||
| $this->actions = $this->table->addColumn(null, 'Actions'); | |||
There was a problem hiding this comment.
This should work fine with new syntax, but looks worse that previous version. It's all about passing that null as first parameter. That's not very intuitive.
There was a problem hiding this comment.
i was evaluating it, and i thought it's better to keep consistency here. surely we have to use null, but at least there is no guessing if it's column name or type.
| $this->class = []; | ||
| } | ||
|
|
||
| if (is_string($this->class)) { |
There was a problem hiding this comment.
Why we need this validation here?
There was a problem hiding this comment.
because sometimes you inject class as ['Button', 'class'=>'big'] and that's wrong. Should be ['Button', 'label', 'class'] if you want.
There was a problem hiding this comment.
ah i see. that way we can pass class as string. ok.
src/Table.php
Outdated
| } else { | ||
| // TODO; set field to null here! |
There was a problem hiding this comment.
why? it should work fine this way too
There was a problem hiding this comment.
ok, actually not sure.
There was a problem hiding this comment.
technically if you do this
$table->addColumn(null, 'Checkbox');
then this colum shouldn't be having any $field created for it. I think it creates some weird field now, it works but taht $field should be null really.
…ure/cleanup-errors
…ure/cleanup-errors
in which our heroes finally clean up test-suite and refactor all those small view constructors.
Merge after #223