Feature Request to Issue filter_searchTrigger #1522

[CiTRO33]

Hey Mottie,

i created a pull request for the enhancement in Issue #1522.

Maybe you have some time to review and test the feature?
I created a fiddle: http://jsfiddle.net/856bzzeL/1568/

Option description:
to enable the feature the option filter_liveSearch must set to false

Possible Values:
bool = default
Number = keycode
String = events ( search, blur )
Objects = { keyCode : Number, alt: bool, ctrl: bool, shift: bool }

Examples:
filter_searchTrigger : false -> same als ['search', 'blur', 13]
filter_searchTrigger : [13] -> Enter
filter_searchTrigger :[ 13, 9] -> Enter and tab - key
filter_searchTrigger : ['blur']
filter_searchTrigger : [{keyCode: 13, ctrl:true}] -> ctrl + Enter

[Mottie]

Hi @CiTRO33!

Thanks for your work on this enhancement. I haven't actually tested the code, but I like the concept so far. Here are a few comments I have on the code I see:

• I think that the "search" event should always be enabled, since it's the developer triggering it on the table cell and not the user.

• Within the filter_searchTrigger, let's use key names like 'enter', 'tab', etc, instead of the key code number. They can be added to the $.tablesorter.keyCodes definition. Then within the new code, use Object.keys (which has good support) for comparisons.

  filter_searchTrigger : [{key: 'enter', ctrl: true}] //=> ctrl + Enter

• filter_searchTrigger should have a default of ['blur', 'enter'], not false because that would imply that there is no search trigger.

• Please follow the code formatting conventions that already exist in the code base. I don't have the .eslintrc set up to match the convention yet, but hopefully it's obvious that there are pretty much spaces everywhere (before and after parenthesis and curly brackets, etc).

[Mottie]

Will this work? If the outer wrapper has event.type === 'search' won't this only capture the 'search' and not the 'blur' event?

[CiTRO33]

in this case only the search event is bind to the table, so the blur event isn't triggered here

[Mottie]

It'd be easier to read if searchTrigger[t] was assigned to a variable:

var trigger = searchTrigger[t]; // for example, name it whatever you want

[Mottie]

The searchTrigger[t].hasOwnProperty('keyCode') isn't really necessary here, it will work with just:

if ( event.which === tskeyCodes[ trigger.key ] ) {

[Mottie]

The above three modifier key checks can be simplified into:

var ctrl= typeof trigger.ctrl === 'undefined' ? true : trigger.ctrl=== event.ctrlKey,
  alt = typeof trigger.alt === 'undefined' ? true : trigger.alt  === event.altKey;
  shift = typeof trigger.shift === 'undefined' ? true : trigger.shift  === event.shiftKey;

[Mottie]

I ran out of time... I'll review the rest of the code later.

[CiTRO33]

Hi,
thanks for the fast response!

i commented the search event out. So it's get triggered always.
But if you look into the fiddle after pressing "Enter" in a filter field, a search event is triggered.
I didn't figured out where is it comming from - but for my usecase its ok that the search is triggered on pressing "Enter".

filter_searchTrigger should have a default of ['blur', 'enter'], not false because that would imply that there is no search trigger.

i agree, but when the default is ['blur', { keyCode: 'enter' } ], and the user init with: filter_searchTrigger: [{keyCode: 'tab'}] the config option get merged as [{keyCode: 'tab'}, {keyCode:'enter'}]
i don't know how the widget options, get merged. - so i decided to go with the value false.

i tried to format the code - but at the moment i have only notepad++ and the github-webeditor to format - sorry

[Mottie]

Hi @CiTRO33!

I'm really sorry for not responding earlier. I've been crazy busy... new job, new baby, etc.

I am trying to catch up with everything now. Would you be willing to update the documentation for this enhancement? Adding it to the demo would be awesome too! 😻

[CiTRO33]

Hi @Mottie ,
i added Docs and Demos (untested) - i hope that helps u to save some time =)