Closed ewu02 closed 9 years ago
@ewu02 this looks really good! I need some time to play with it, but code looks good. And yes you are right we should look into decoupling some of this code inside the components to avoid duplication.
Since we are building with browserify now, we can actually start to move the components inside reactGrid.jsx into their own files and just have reactGrid.jsx require them.
We can then create an NgReactGridSearchService/Manager class that encapsulates this functionality.
Let me play with it tonight and i'll merge this in. Thanks again.
Jose
Cool. There are additional issues, such as not being able to handle data change. Currently I'm not sure how to cleanly resolve that issue. NgReactGridReactManager.searchValues would need to be reset after data change but not from filtering, which is tethered to it. NgReactGrid.prototype.updateSearch seems like a good place, but it's not currently used and I'm not sure what kind of plans you have for it.
Column search method could be much more optimized, though it might involve significant changes to setSearch and deepSearch methods.
Are 'input's in <input type='input' .../> placeholders for now?
@ewu02 Thanks for all of this btw, very good initiative. I had some time to take a look today.
I have posted some comments on commit https://github.com/ewu02/ngReactGrid/commit/3c9ebb7ee567074f24d25429f4c31e90c510a626#diff-39b63cc0a24520f37bdffb1eb8c5c109R203
Also, I was thinking that we should make the search fields show on the click or hover of a filter icon that we can put on the header cell next to the sort if columnSearch: true. This will keep the UI clean. We should probably mock this up.
One last thing, the search seems to behave funky when searching on two columns at the same time. Does the same happen for you?
I think this is looking great. It's going to be a good feature.
Let me know what you think. Oh and what do you mean about
Jose
Thanks for the inputs. Forgot about github markdowns. I meant to say:
Are 'input's in <input type="input" placeholder="Search..." ... />
placeholders for now until there are user defined types? Come to think of it, you probably meant it to be <input type="text" ... />
Multiple columns search seems to behave fine provided that initial data don't change. I think you may have tested it using my branch with commit 'Reset search values on data change.' That one has the funky multiple column search behavior, and I have since reverted it.
Good idea on hiding the search input. I'm thinking of having the input search field displayed above header on filter icon click. Does that seem good to you?
Filter icon should be search icon for consistency, though I think filter is a more appropriate term.
@ewu02 you are correct that should be type="text"
I'm going to work on a mockup this weekend and share it with you.
@ewu02 I haven't forgotten about this btw, I am having a hectic couple of weeks at work so I hope to be able to finalize this with you very soon.
That's fine, take your time. I've been busy at work as well. Thanks for the update.
@ewu02 I'm flying back to Miami next weekend, i should have some time to finish the mockup.
Moving to a different branch
Thinking of creating a method in NgReactGridReactManager that is similar to getObjectPropertyByString that gets the base key/column name for arbitrarily nested data column reference. Currently data source must have unique key names throughout all levels.
Styling and sizing is piggybacked off ngGridHeaderCell. Wonder if we should decouple some of its internal methods so ngReactGridColumnSearch doesn't need to duplicate them when moved out of thead.
Any suggestions would be most welcome.