feat: Hide columns on list tasks #164
Conversation
refs: 2198 issues:
Dane-2pi
changed the title
Add feature to hide columns from view in listtasks partial update to fit code standards refs: 2198 issues:
…ware/cmfive-core into feature/SortProjectTasks
| $w->ctx("redirect", $params['redirect']); | ||
|
|
||
| $w->ctx("hide_filter", array_key_exists('hide_filter', $params) ? $params['hide_filter'] : false); | ||
|
|
||
| $w->ctx("hide_columns", array_key_exists('hide_columns', $params)? $params['hide_columns'] : false); |
There was a problem hiding this comment.
I would just have the one parameter "columns_to_hide" because it's existing or absence can trigger the relevant code further down.
There was a problem hiding this comment.
There was a reason we wanted 2 variables though. We manipulate the array of columns to hide and so I figured it would be cleaner to separate concerns.
There was a problem hiding this comment.
That's OK, you can send a new parameter into the ctx but it doesn't have to be a second parameter to the partial. Easy to write:
$w->ctx("hide_columns", array_key_exists("columns_to_hide", $params));
We shouldn't expose internal workings to the external interface.
| $table_header = array("Title", "Created By", "Assigned To", "Group", "Type", "Priority", "Status", "Due"); | ||
| $table_data = array(); | ||
| if ($hide_columns) |
There was a problem hiding this comment.
As per my comment above, this parameter is unnecessary because we can just check the existence of "columns_to_hide" to achieve the same effect.
| $taskgroup = null; | ||
| if (!empty($params['task_group_id'])) { | ||
| if (!empty($params['task_group_id'])) | ||
| { |
There was a problem hiding this comment.
Why are all the brackets of the if statements on new lines? This doesn't match our code standard... maybe the code sniffer is configured wrong?
|
@adam-buckley , is this good to be merged and closed out? Looks like we were both happy with it some time ago |
|
has been merged in some time ago. closing |
Checklist
Description
Add feature to hide select columns in the listtasks partial
Changelog
refs: 2198
issues:
Other Information
Docs pull request: