Improvements for Tables, CRUDS and few other areas. - #277
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #277 +/- ##
============================================
- Coverage 75.64% 75.54% -0.1%
- Complexity 1131 1132 +1
============================================
Files 63 63
Lines 2800 2691 -109
============================================
- Hits 2118 2033 -85
+ Misses 682 658 -24
Continue to review full report at Codecov.
|
DarkSide666
left a comment
DarkSide666
left a comment
There was a problem hiding this comment.
Overall this is good update.
I don't see afterRow hook implementation. That would take away need to mess with model values in beforeRow hook to add row after current one.
On the other hand - that's another hook to call and we can work around need for it like you have shown in demo - by changing model values. Not nice workaround, but .... it works.
See few more review comments in code.
| $table->template->appendHTML('Body', '<tr class="center aligned"><td colspan=2>This goes above row with ID=2 ('.$table->current_row['action'].')</th></tr>'); | ||
| } elseif ($table->current_id == 3) { | ||
| $table->renderRow(); | ||
| $table->model->set(['action'=>'manually injected row after Tax', 'amount'=>0]); |
There was a problem hiding this comment.
this is quite hackish way to do this, but it works of course :)
| return View::renderView(); | ||
| } | ||
|
|
||
| public function renderRow() |
There was a problem hiding this comment.
No comments added for this method.
| } | ||
| $field = $this->model->hasElement($name); | ||
| foreach ($columns as $column) { | ||
| if (!method_exists($column, 'getHTMLTags')) { |
There was a problem hiding this comment.
please simplify this to:
if (method_exists($column, 'getHTMLTags')) {
$html_tags = array_merge($column->getHTMLTags($this->model, $field), $html_tags);
}
No need for negation and continue; here.
There was a problem hiding this comment.
i dont understand, please propose through a separate PR.
| $rows = 0; | ||
| foreach ($this->model as $this->current_id => $tmp) { | ||
| $this->current_row = $this->model->get(); | ||
| $this->hook('beforeRow'); |
There was a problem hiding this comment.
I think hook should be after updateTotals(), because if you change model values (like in example where you add row after current one), then totals will be messed up too.
On the other hand - sometimes it can be useful to add some additional conditions on updateTotals().
Also i think it would be great if we add ability to return false from beforeRow hook and in that case row will not be rendered (and not added to totals) at all. In that case it is good to have hook before updateTotals().
romaninsh
changed the title
2 participants
Tables are good for outputting data from a model, but sometimes you need to add that extra row, some sub-header or subtotal.
This adds example on how you can do that and adds two new hooks into table:
beforeRowandafterRow