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

Reset button optional #30

Merged
merged 1 commit into from
Mar 15, 2014
Merged

Conversation

gator92
Copy link
Contributor

@gator92 gator92 commented Feb 26, 2014

There are some use cases where you would not want the reset button included by default. For example, a select menu where the first choice is "Any" or "All" and selecting that would reset the filter. In this pr, passing false to the filter_reset_button_text options would disable the reset button.

@vedmack
Copy link
Owner

vedmack commented Feb 26, 2014

Thanks, will take a look at it later on, I didn't implement it myself because it was possible to achieve it using css display:none :)

vedmack added a commit that referenced this pull request Mar 15, 2014
@vedmack vedmack merged commit 0923b48 into vedmack:master Mar 15, 2014
@vedmack
Copy link
Owner

vedmack commented Mar 15, 2014

merging and replacing into

filter_reset_button_text !== false

still not sure if to add new property like show_filter_reset_button instead of abusing filter_reset_button_text

@gator92
Copy link
Contributor Author

gator92 commented Mar 17, 2014

@vedmack Thanks for merging. I see what you mean though. Maybe it would be better to expand the setting with something like this:

{filter_reset_button : 
    {show : true, text : 'x'}
}

@vedmack
Copy link
Owner

vedmack commented Mar 17, 2014

Can't see a scenario in which one will use both of the options, the common usage is set some alternative text or hide , so forcing user to fill both of the values and using an object instead of a primitive is redundant. I guess that for now the current settings are fine.

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