column-sortable icon indicating copy to clipboard operation
column-sortable copied to clipboard

@sortablelink parameter #3 not updating and using current value

Open seche opened this issue 6 years ago • 4 comments

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

seche avatar Feb 24 '20 13:02 seche

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?

Kyslik avatar Mar 01 '20 19:03 Kyslik

@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 avatar Mar 01 '20 22:03 seche

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

seche avatar Mar 10 '20 18:03 seche

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.

LFavier avatar Oct 22 '21 08:10 LFavier