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

FEATURE REQUEST - Text filters with dynamic regex switch #490

Merged
merged 7 commits into from
Sep 5, 2018
Merged

FEATURE REQUEST - Text filters with dynamic regex switch #490

merged 7 commits into from
Sep 5, 2018

Conversation

misanek
Copy link
Contributor

@misanek misanek commented Aug 22, 2018

added requested feature #489

jsfiddle with example : http://jsfiddle.net/xvkgu4La/18/

@vedmack
Copy link
Owner

vedmack commented Sep 3, 2018

Thanks!
But please note that PR should be applied to the yadcf/src/jquery.dataTables.yadcf.js (to the latest beta file which is the most updated version of yadcf

@misanek
Copy link
Contributor Author

misanek commented Sep 4, 2018

Hi @vedmack ,
as I was waiting for you to accept this PR, I wanted to use my fork in a project's package.json, so that's why I changed the release files as well ... I can revert other files and left only the beta file.

@vedmack
Copy link
Owner

vedmack commented Sep 4, 2018

@misanek , I will accept it after you modify only the beta js file only (its ok to change release css file)

@misanek
Copy link
Contributor Author

misanek commented Sep 4, 2018

@vedmack , reverted commit, it should be fine now

@vedmack
Copy link
Owner

vedmack commented Sep 4, 2018

@misanek first of all wanted to tell that I really appreciate your PR,

I just reviewed your code, this feature usage shouldn't be different than the exclude feature (they both have a similar UI / therefor should have similar usage from API), what I mean is that enabling feature by setting its text is wrong IMO, you should have a default value (with ability to customize) for the label and be able to turn the feature on/off (off default) by setting its boolean property.

You can do a "search" for ".exclude" in the yadcf js file and to get the idea of what I'm talking about.

See exclude feature on the showcase

@vedmack vedmack merged commit 6c599d1 into vedmack:master Sep 5, 2018
@vedmack
Copy link
Owner

vedmack commented Sep 5, 2018

Thanks @misanek ! Looks great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants