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.

Sign up for GitHub

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

Jump to bottom

@sortablelink parameter #3 not updating and using current value #144

Open
seche opened this issue Feb 24, 2020 · 4 comments · May be fixed by #145
Open

@sortablelink parameter #3 not updating and using current value #144

seche opened this issue Feb 24, 2020 · 4 comments · May be fixed by #145
Assignees
@Kyslik
Labels
needs information

Comments

@seche
Copy link

seche commented Feb 24, 2020

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 seche linked a pull request Feb 24, 2020 that will close this issue
Open
@Kyslik
Copy link
Owner

Kyslik commented Mar 1, 2020 •
edited
Loading

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 Kyslik self-assigned this Mar 1, 2020
@seche
Copy link
Author

seche commented Mar 1, 2020 •
edited
Loading

@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
Copy link
Author

seche commented Mar 10, 2020

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

@LFavier
Copy link

LFavier commented Oct 22, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Kyslik Kyslik

Labels
needs information
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update SortableLink.php seche/column-sortable
3 participants
@seche @Kyslik @LFavier