Column filters #185
Column filters #185
Conversation
|
Hi, did you have any chance of going through this? I was waiting for it to be merged to add the new datepicker editor and filter to the table I added in this PR. |
|
|
||
| @Component({ | ||
| selector: 'completer-filter', | ||
| styles: [` |
There was a problem hiding this comment.
@damnko, could you move styles to a separate file?
There was a problem hiding this comment.
Sure, could you tell me why you prefer to move them to a separate file even if they are just a few lines long?
There was a problem hiding this comment.
/deep/ highlights in IDE like an error. 😃
| } | ||
|
|
||
| ngOnInit() { | ||
| this.changesSubscription = Observable.fromEvent(this.filterEl.nativeElement, 'change') |
There was a problem hiding this comment.
I don't think that will work since the value of the checkbox does not change.. what do you think? Maybe I could check that simply with the (chage) event but I wanted to know why you would prefer that over the Observable pattern. Just for personal knowledge and for future reference.
Thanks
There was a problem hiding this comment.
@damnko, my example uses ngModel and it is more angular style, simply for reading and has fewer code rows.
There was a problem hiding this comment.
Thanks for your feedback, I would like to share with you a couple of thoughts:
- Using Observables we could leverage methods as
.distinctUntilChanged()and.debounceTime()respectively to avoid triggering when the value is unchanged and to trigger the search only after a defined amount of time, this latter thing was previously done by setting and removing asetTimeout(). Please let me know if I should remove this delay feature or if I should use your previoussetTimeout()method. - I am not sure that
[(ngModel)]or(ngModelChange)will work since the value of the checkbox does not change, only it'scheckedattribute changes... but I will give it a try!
There was a problem hiding this comment.
We are not against Observables for sure! :) It's just might be not a good practice to use native JS API events when angular provides some wrappers around it. Moreover, I fell like we can combine both observables and angular way, let me get back to you with a link in a moment.
There was a problem hiding this comment.
Probably something like this http://stackoverflow.com/a/34656612? But yeah, in this case, I'm not sure this would be the best solution to use FortControl, @lexzhukov do you have some better ideas?
There was a problem hiding this comment.
FormControl looks great too.
There was a problem hiding this comment.
Actually I don't know why I thought that [(ngModel)] would not have worked with checkbox type input... it works and I will follow your suggestions. Thanks!
|
Hey @damnko, sorry for making you wait. We just looked into the there is a question about the checkbox filter. So the issue is that by default the checkbox filter in not checked, but not checked state means FALSE, so by the meaning the table should show only the rows which have these values equal to FALSE. But that doesn't make such sense. Probably at this stage, we shouldn't implement the checkbox and leave it for now or don't implement such filter at all? I know you spent your time on it, but we haven't thoughts about this till we saw it. What do you think? |
|
Hi, I was replying to one of @lexzhukov comments, but to make it quick: at the moment I have implemented a "reset" button to reset the checkbox value. So it starts as "empty" and then when clicked, being it either checked or unchecked, this "reset" text appears and lets you reset to the initial "empty" value. It seems to work, let me know if you like the solution, thanks. |
|
@damnko yeah, I saw this, but still this makes it a bit strange, when at first "empty" mean nothing, and after a couple of click "emtpy" means "false". It just doesn't feel right from the UX standpoint, don't you think? |
|
Yes I agree it's not very consistent but personally I wouldn't care much as long as it works. I perfectly understand your point though and it won't be a problem for me to simply remove the functionality, I did not spend much time on it ;) so no worries. The only workaround that comes to my mind atm would be to highlight in some way the columns that are being filtered so it will be clearer than seeing a little "reset" text. Just thinking out loud, don't even know if it would be a viable solution... |
|
@damnko well, probably we can think of some solution of this sort later, at the moment - let's probably leave it as it is, as basically we don't force someone to use it, and it's possible to select any other available filter event the column is a type of 'checkbox'. |
|
Hopefully everything should be ok now. Thanks again for the really interesting tip on |
|
@lexzhukov probably we are ready to merge? |
|
@nnixaa yes, we are ready. |
Sorry for the late update: I added column filters of type
checkbox,completer,select.Let me know if everything seems fine.
Thanks