Add parameter in addCheckbox to prepend or append the checkbox column.
When I try to add a checkbox column as the first column as following it doesn't show the checkbox column at all, if I put theaddCheckbox() aftercolumns it comes as the last column.

  return $this->builder()              ->addCheckbox()               ->columns($this->getColumns())

Add parameter in addCheckbox to prepend or append the checkbox column

6555c25

Add parameter in addCheckbox to prepend or append the checkbox column.

Copy link

ContributorAuthor

karmendra commentedFeb 5, 2018

Can someone review this please, CI has failed, its suggestions are not what we would expect in the code. Can someone confirm if we are waiting for CI to Pass before this PR is visible for review?

Copy link

Contributor

Namoshek commentedFeb 26, 2018

Just had a look at your PR and to me it looks fine (technically and style). I can confirm that the suggested StyleCI changes are not related to the changes of yours.

I just have one suggestion for an improvement: Maybe make the new parameter@param bool|int to also allow for positional inserts. Consider something like this:

$this->builder()     ->columns([         ['data' =>'id','title' =>'ID'],         ['data' =>'name','title' =>'Name'],         ['data' =>'email','title' =>'Email'],         ['data' =>'telephone','title' =>'Telephone'],     ])     ->addCheckbox(['data' =>'is_admin','title' =>'Admin?'],2);

which would hopefully give you something like

 ID | Name | Admin? | Email            | Telephone-----------------------------------------------------  1 | Luke |    Yes | luke@example.com | 1234 567890

where2 is the 0-based index of where the checkbox column should go.

Obviously this would require some edge case handling to cover when there is a too high index for example.

Copy link

Owner

yajra commentedFeb 27, 2018•
edited
Loading

Sorry for overlooking this, a bit busy on mobile dev and cannot find time yet for open source projects. Anyways,~~will fix the StyleCI and~~ will try to review this within the week. Thanks!

Update --
StyleCI issue seems like like due to targeted branch is master. Updated the PR to target 3.0.

yajra changed the base branch frommaster to3.0

February 27, 2018 01:27

yajra requested changes

Feb 27, 2018

View reviewed changes

src/Html/Builder.php Outdated

		],$attributes);
		$this->collection->push(newColumn($attributes));

		if($prependColumn) {

Copy link

Owner

yajraFeb 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Please add a space afterif ($prependColumn)

Copy link

ContributorAuthor

karmendraFeb 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Added a space after if

Copy link

ContributorAuthor

karmendra commentedFeb 28, 2018•
edited
Loading

@Namoshek Very good suggestion, but I am not sure how I can insert an item in Collection at specific index. My quick search showed that we can use splice prepend/push and union functions to do so, but felt that's really not good way to do it. Any other suggestion do let me know.

Code Style updated

296be4f

Added a missing space after if statement

Copy link

Contributor

Namoshek commentedFeb 28, 2018•
edited
Loading

Yes,$collection->splice($index, $length, $elementsToInsert) is the way to go, I think. Although you could also build a new collection from ground if that's easier for you (due to the very limited amount of entries for the collection, this would not be a performance issue either).

I think something along these lines could be a potential solution (completely untested though):

/** * Add a checkbox column. Prepends the column at the beginning, * appends it at the end or inserts it at a specified index. * * @param  array $attributes * @param  bool|int $position true to prepend, false to append or a zero-based index for positioning * @return $this */publicfunctionaddCheckbox(array$attributes = [],$position =false){$column =newColumn(array_merge(['defaultContent' =>'<input type="checkbox"' .$this->html->attributes($attributes) .'/>','title'          =>$this->form->checkbox('','',false, ['id' =>'dataTablesCheckbox']),'data'           =>'checkbox','name'           =>'checkbox','orderable'      =>false,'searchable'     =>false,'exportable'     =>false,'printable'      =>true,'width'          =>'10px',    ],$attributes));if ($position ===true) {$this->collection->prepend($column);    }elseif ($position ===false ||$position >=$this->collection->count()) {$this->collection->push($column);    }else {$this->collection->splice($position,0, [$column]);    }return$this;}

If you are curious what the equivalent of theif-elseif-else part is:

if (is_int($position)) {if ($position >=$this->collection->count()) {$this->collection->push($column);    }else {$this->collection->splice($position,0, [$column]);    }}else {if ($position) {$this->collection->prepend($column);    }else {$this->collection->push($column);    }}

But I find the above version more readeable and also a bit more logic when thinking about it twice. It makes use of implicit type checks though (=== vs.is_int()).

Adding checkbox by column position

2759fd9

Adding checkbox by column position ( zero indexed i.e. index starting from 0)

Copy link

ContributorAuthor

karmendra commentedMar 4, 2018

@yajra why should the PR target be 3.0 and not master? I am not able to install the changes in Laravel 5.5 and above. I get following error

vagrant@homestead:~/Code/traknpay$ composer updateLoading composer repositories with package information                                                 Updating dependencies (including require-dev)         Your requirements could not be resolved to an installable set of packages.  Problem 1    - Can only install one of: laravelcollective/html[5.4.x-dev, v5.6.3].    - Can only install one of: laravelcollective/html[5.6.x-dev, 5.4.x-dev].    - Can only install one of: laravelcollective/html[5.6.x-dev, 5.4.x-dev].    - Can only install one of: laravelcollective/html[v5.6, 5.4.x-dev].    - Can only install one of: laravelcollective/html[v5.6.1, 5.4.x-dev].    - Can only install one of: laravelcollective/html[v5.6.2, 5.4.x-dev].    - Can only install one of: laravelcollective/html[v5.6.3, 5.4.x-dev].    - yajra/laravel-datatables-html dev-addCheckbox requires laravelcollective/html 5.4.* -> satisfiable by laravelcollective/html[5.4.x-dev].    - Installation request for yajra/laravel-datatables-html dev-addCheckbox -> satisfiable by yajra/laravel-datatables-html[dev-addCheckbox].    - Installation request for laravelcollective/html ^5.6 -> satisfiable by laravelcollective/html[5.6.x-dev, v5.6, v5.6.1, v5.6.2, v5.6.3].

any suggestions?