[Enhancement] Fixed issue #447 (Search box - Default Text) #852
Conversation
Added searchPlaceholder option
|
I think this is a useful option, however, I'll have to give some thought to the API. |
gabrielliwerant
added
the
needs review
There was a problem hiding this comment.
This feature has been needed for a long time, great job in providing that.
My only nitpick is that I think we can fold this option into an existing example to prevent example bloat.
My concern with the API is that, as it grows in size, it becomes more difficult to find what options are available. Some of that could be helped by better documentation, but there might also be a case to combine some options into their own object/namespaces as we do with some others, however, I don't think that's necessary here, at least not yet. Just wanted to explain my thinking on it.
README.md
Outdated
| @@ -166,6 +166,7 @@ The component accepts the following props: | |||
| |**`filter`**|boolean|true|Show/hide filter icon from toolbar | |||
| |**`search`**|boolean|true|Show/hide search icon from toolbar | |||
| |**`searchText`**|string||Initial search text | |||
| |**`searchPlaceholder`**|string||Search text placeholder. [Example](https://github.com/gregnb/mui-datatables/blob/master/examples/customize-search-placeholder/index.js) | |||
There was a problem hiding this comment.
It might be overkill to add a whole new example just for this feature. What if we added it to customize-search instead?
There was a problem hiding this comment.
Yes true, I agree
I thought it was a convention to add examples for everything haha but it makes sense not to.
I'll do the changes today :) Thanks for your time.
There was a problem hiding this comment.
No worries! There's no hard convention for it, it's more a matter of, is there enough new functionality to justify a new example, or can it be squeezed in elsewhere. It's a more a feeling. :p
Thank you for working on this!
There was a problem hiding this comment.
Haha no problem, I'm open for more issues to work on, I can just pick any non assigned issue, correct?
There was a problem hiding this comment.
The best ones would be those marked "bug" or "help wanted" that haven't yet been assigned. If in doubt, @-me in the comments and ask what I think and I'll get back to you within a day.
gabrielliwerant
added
needs work
new feature
and removed
needs review
Deleted examples/customize-search-placeholder Added searchPlaceholder to examples/customize-search instead
Updated documentation Example link
table option was merged in release 2.10.0 gregnb/mui-datatables#852
fix #447
Added
searchPlaceholderoption toTableSearch.js