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

Cookie extension should not override data-search-text property value #4984

Closed
Vity01 opened this issue Apr 17, 2020 · 9 comments · Fixed by #5583
Closed

Cookie extension should not override data-search-text property value #4984

Vity01 opened this issue Apr 17, 2020 · 9 comments · Fixed by #5583
Labels
confirmed Issues that have been confirmed with a reduced test case and identify a bug. cookie Issues for the cookie extension. feature Issues asking for a new feature to be added, or an existing one to be extended or modified. has PR Issues that has been fixed with a PR.

Comments

@Vity01
Copy link

Vity01 commented Apr 17, 2020

When I set initial value for the input search, the result input value is always set to the empty string.
(eventhough I don't want the searchInput text cookie to be used)
Table properties:

data-search-text="mySearchValue"
data-cookies-enabled="['bs.table.columns']"

The core of the problem resides in the cookie extension. It always passes empty string no matter other property (cookie is not defined, data-search-text is set)

  // this.options.searchText ... mySearchValue
  // searchTextCookie  is null
   this.options.searchText = searchTextCookie ? searchTextCookie : '';

I suppose that correct behavior should be something like this (pseudocode):

  if (cookieEnabled('bs.table.searchText') && this.options.searchText == '') {
        this.options.searchText = searchTextCookie ? searchTextCookie : '';
  }

Used version: 1.16.0

@UtechtDustin UtechtDustin added confirmed Issues that have been confirmed with a reduced test case and identify a bug. cookie Issues for the cookie extension. labels Apr 17, 2020
@UtechtDustin
Copy link
Collaborator

UtechtDustin commented Apr 17, 2020

Thank you for the report.
I dont checked the whole code for now but i guess the best solution is to check if a search from cookie exists, if not it should be check if a the searchText option is used.
If that option is used the text from that option should be used, if not -> emptystring
Example Code:

let searchText = '';

if (this.options.searchText !== '') {
   searchText = this.options.searchText
} else if (searchTextCookie != '') {
   searchText = searchTextCookie
}
//set the searchtext

@Vity01
Copy link
Author

Vity01 commented Apr 20, 2020

@UtechtDustin You should ignore the cookie value even it already exists if the table options (data-cookies-enabled) ignore it as well.

@UtechtDustin
Copy link
Collaborator

I dont think so, because if somebody use a default value he want to filter for a default dataset.
If somebody change that filter he has a reason for that.
Maybe we could add an option to switch between cookie > default and default > cookie.
@wenzhixin @djhvscf what do you think ?

@Vity01
Copy link
Author

Vity01 commented Apr 20, 2020

@UtechtDustin I don't understand your example with filters. If I don't want a cookie support for the given property (search-text in this case) why it should be the cookie used?

@365dayes
Copy link

Able to replicate as well with cookies. Doesn't respect the data-search-text value. Also data-filter-control as well also exhibits this issue.

@djhvscf
Copy link
Collaborator

djhvscf commented Jul 31, 2020

I dont think so, because if somebody use a default value he want to filter for a default dataset.
If somebody change that filter he has a reason for that.
Maybe we could add an option to switch between cookie > default and default > cookie.
@wenzhixin @djhvscf what do you think ?

I think we can add an option like data-cookie-override-search='true' or something in order to define the precedence

@djhvscf djhvscf added the feature Issues asking for a new feature to be added, or an existing one to be extended or modified. label Jul 31, 2020
@365dayes
Copy link

Looks like reverting to 1.15.5 for data filter control minimizes the issue. if that helps thanks guys
https://unpkg.com/bootstrap-table@1.15.5/dist/extensions/filter-control/bootstrap-table-filter-control.min.js

@djhvscf
Copy link
Collaborator

djhvscf commented Aug 2, 2020

Thanks @365dayes will compare the current and old versions.

@djhvscf
Copy link
Collaborator

djhvscf commented Nov 23, 2021

Example with fix: https://live.bootstrap-table.com/code/djhvscf/9692

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed Issues that have been confirmed with a reduced test case and identify a bug. cookie Issues for the cookie extension. feature Issues asking for a new feature to be added, or an existing one to be extended or modified. has PR Issues that has been fixed with a PR.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants