Kyslik/column-sortable

@sortablelink parameter #3 not updating and using current value

Opened this issue · 4 comments

seche commented

Noticed a simple bug, same as #88 but since the fix wasn't applied, I figured it needed to be raised again.

When the parameter is already set/active in your current URL, whatever you put in the 3rd parameter @sortablelink('address', trans('fields.address'), ['filter' => 'active, visible']) doesn't override the value.

So if your URL http://127.0.0.1:8000/home?foo=bar&sort=name&direction=asc where foo= bar. Using @sortablelink('address', trans('fields.address'), ['foo' => 'manchu']) will still give you a URL with foo = bar.

The fix is basically swapping the variables $persistParameters and $queryParameters on line 241 of SortableLink.php $queryString = http_build_query(array_merge($persistParameters, $queryParameters, [ so that what you pass @sortablelink overrides what is currently set.

I understand that you can explicitly override the Get variable @sortablelink('name', __('head.name'), ['var' => request()->get('var', 'bar')]) but one should assume that setting a variable in the function call should work. And it's a super simple fix.

Thanks

Kyslik commented

Thank you for detailed issue! :)

According to the docs:

array() parameter (3rd) is default (GET) query strings parameter

Emphasis mine.

So it is correct as it is, am I right?

seche commented

@Kyslik yes but if the get variable name is already set, it doesn't override the sortable link to use the one in parameter #3 in the function due to the order they were originally set in the code of the function.

For instance, I use a variable tabs with sortable tables but if tabs is already set in the get variable then it sets the same tabs value in all my sortable links on all my tabs and ignoring what values I set in the function call.

seche commented

Can we please merge #145? Really seems like a no brainer. Thanks

LFavier commented

I agree with @seche.
This would for example allow users to have two sortable arrays on one page with the third parameter indicating which sort to use.
Sadly right now I can't find a way to implement this otherwise, and it devalues the use of this package quite a bit for me.

Please correct me if there is another way.

  • 👍1