Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include column title with request #52

Closed
robbielove opened this issue Mar 2, 2017 · 7 comments
Closed

include column title with request #52

robbielove opened this issue Mar 2, 2017 · 7 comments

Comments

@robbielove
Copy link

robbielove commented Mar 2, 2017

Hi,

I am trying to show the sort criteria and It would be nice if the 'title' used in the @sortablelink() was passed in the request in addition to sort and order so that it can be shown.

I am currently using ucfirst($request->sort), however this only works if the column name is the same as the title which in some cases will produce differences

@Kyslik
Copy link
Owner

Kyslik commented Mar 2, 2017

You can send it yourself (see https://github.com/Kyslik/column-sortable#blade-extension)

@sortablelink('name', 'Username', ['title' => 'whatever you want'])

should generate:

/page?title=whatever%20you%20want

Also take a look at https://github.com/Kyslik/column-sortable/blob/L5.4/src/config/columnsortable.php#L67 you can set formatting_function.


If that is not what you meant explain further, otherwise just close :).

@robbielove
Copy link
Author

I have used that to pass the request input (other filters etc) however it becomes problematic because the title parameters need to be merged with this request data array

I have tried
@sortablelink('id', 'ID', [$request->except(['title']),'title' => 'ID'])
and
@sortablelink('id', 'ID', array_add($request->except(['title']),'title','ID'))

which works for the first one but then somehow it will continue to use the first column selected (not removing the current title and replacing with the new one)

I have tested creating another link with just:
{{ route('users.index', array_add($request->except(['title']),'title','ID')) }}
{{ route('users.index', array_add($request->except(['title']),'title','Name')) }} etc,
as the href which works perfectly however obviously doesn't get the sortable value.

The sortable link for all headers still contains whatever the $request->title value is:
@sortablelink('name', 'Name', array_add($request->except(['title']),'title','ID'))
@sortablelink('name', 'Name', array_add($request->except(['title']),'title','Name')) etc

Which leads me to believe your function includes the wrong value where there are keys that match the request input. eg. if $request->title is set to TEST then the sortable link will contain:
/users?sort=id&order=asc&active=1&roles=member&title=TEST
/users?sort=name&order=asc&active=1&roles=member&title=TEST - from: @sortablelink('name', 'Name', array_add($request->except(['title']),'title','Name')) - but should show: /users?sort=slug&order=asc&active=1&roles=member&title=Name

I was kinda hoping avoid the double entry of the title, and having to merge the request array and exclude the request title adds to this burden. Although this may not be so bad if the link function worked as it should

using L5.2 branch

@Kyslik
Copy link
Owner

Kyslik commented Mar 3, 2017

I kind of still do not understand what you are trying to achieve.

So you need for some kind of reason get parameter title so you can work with it later in controller?

I am trying to show the sort criteria and It would be nice if the 'title' used in the @sortablelink() was passed in the request in addition to sort and order so that it can be shown.

Gottcha! Now I understand.


What about

@sortablelink('name', 'Name', ['title' => 'Name'])

I might try to add config value in order to set boolean value if user want to inject title in request.

@robbielove
Copy link
Author

I might try to add config value in order to set boolean value if user want to inject title in request.

that is pretty much what I was after
@sortablelink('name', 'Name', $request->except(['title']), true))
@sortablelink(column, title, pass parameter, pass/merge title))

Kyslik added a commit that referenced this issue Mar 3, 2017
Kyslik added a commit that referenced this issue Mar 3, 2017
Kyslik added a commit that referenced this issue Mar 3, 2017
Kyslik added a commit that referenced this issue Mar 3, 2017
Kyslik added a commit that referenced this issue Mar 3, 2017
@Kyslik
Copy link
Owner

Kyslik commented Mar 3, 2017

Its done, I just push new release after test pass.

@robbielove
Copy link
Author

I have implemented your changes and it seems to be working perfectly, thank you so much 😊

@Kyslik
Copy link
Owner

Kyslik commented Mar 3, 2017

No problem. Have a good artisan adventures.

@Kyslik Kyslik closed this as completed Mar 3, 2017
Kyslik added a commit that referenced this issue Mar 12, 2017
* master:
  Apply CI
  Update license
  Implement #52, introduce new configuration key-value pair
  Update travis and readme badge
  Change min-stability to dev and version of testbench
  Change min-stability to dev
Kyslik added a commit that referenced this issue Mar 14, 2017
* L5.3:
  Add test
  Extract methods
  Allow columns that have value of 0 to be present
  Update license
  Implement #52, introduce new configuration key-value pair
  Update readme badge, update travis.yml, simplify queryJoinBuilder()
  Update readme
  Branch out for specific Laravel version

# Conflicts:
#	README.md
#	composer.json
#	src/ColumnSortable/SortableLink.php
#	tests/SortableLinkTest.php
Kyslik added a commit that referenced this issue Mar 14, 2017
* L5.3:
  Add test
  Extract methods
  Allow columns that have value of 0 to be present
  Update license
  Implement #52, introduce new configuration key-value pair
  Update readme badge, update travis.yml, simplify queryJoinBuilder()
  Update readme
  Branch out for specific Laravel version

# Conflicts:
#	.travis.yml
#	README.md
#	composer.json
#	src/ColumnSortable/SortableLink.php
#	tests/SortableLinkTest.php
Kyslik added a commit that referenced this issue Mar 14, 2017
* L5.2:
  Update license
  Implement #52, introduce new configuration key-value pair
  Update readme badge, update travis.yml, simplify queryJoinBuilder()
  Readme update
  Branch out for specific Laravel version

# Conflicts:
#	.travis.yml
#	README.md
#	composer.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants