6pac / SlickGrid

A lightning fast JavaScript grid/spreadsheet
https://github.com/6pac/SlickGrid/wiki
MIT License
1.82k stars 424 forks source link

feat: add the ability to receive extra filter arguments #1007

Closed AlexandrChazov closed 5 months ago

AlexandrChazov commented 5 months ago

It would be very handy for developers to have ability to receive extra arguments of filter function. For example when filter is applyed we can:

Previously there was ability to receive extra arguments of filter function by using dataView.filters property and much featers have already been written by using it. Now we need a way to keep these features working.

ghiscoding commented 5 months ago

I don't see the utility of this since you are the developer who provides these filter arguments, why do you need a GETTER for? I mean you obviously know the filter arguments since you provided them, so what's the point? Is that supposed to be used by another file outside of where you instantiate the DataView?

AlexandrChazov commented 5 months ago

Yes, you are right. The getter will be convinient to use outside the place where we assign filter arguments. Let's say we have filters in a Header and a table in a Main section. They are different React components. We subscribed to grid.onAddNewRow event and passed a callback which should add a new row satisfying to the new filter arguments so that it isn't hidden. In order to do this we need to pass filter arguments through the context or by the help of a state manadger what will cause component to rerender. We may also need to unsubscribe from grid.onAddNewRow even and subscribe to it one more time with new filter arguments. It would be much easier to use the getter inside the callback to get new filter arguments. Same with custom Editors which know nothing about filters (e.g. date Editor) and should set initial year relying on a year filter value. It's not convinient to reasign column Editor every time we changed filters. Much more easier to call DataView.getFilterArgs() and we are done ))

ghiscoding commented 5 months ago

yeah ok I guess we could accept that, also for your info I'm not sure if you aware of another repo of mine Slickgrid-React it's based on Slickgrid-Universal which I also maintain. It's a larger project because it's more of an all-in-one package (all assembled) as opposed to SlickGrid which is more like an IKEA product (you need to assemble it yourself). Also note that I'm not a React developer (someone else created the repo and transferred it to me because I maintain a few other platforms)

AlexandrChazov commented 5 months ago

Yes, I'm aware of Slickgrid-React repo, I have already tried it but faced an issue with SASS and I also had thought that as well as slickgrid-es6 library they may stop updating it. Maybe I should take another look at it.

ghiscoding commented 5 months ago

@AlexandrChazov I'm not sure if you saw but I left a comment to fix (or provide feedback) above, please review. As for SASS, I'm not sure what problems you had but I literally copied my implementation from Slickgrid-Universal into SlickGrid, so it should be the same.

Also, one of the reason I keep updating SlickGrid (this repo), even though I'm no longer using it, is because it has a larger user base and I needed to convert SlickGrid & SlickDataView to TypeScript first before I could copy it over to Slickgrid-Universal. So because SlickGrid has more users, it was good to test it here before sending it over to Slickgrid-Universal later. There are also plenty of users who prefer the SlickGrid IKEA approach too, it's smaller than Slickgrid-Universal for basic grids but you get a lot less features

AlexandrChazov commented 5 months ago

I'm sorry, I don't know how to work with comments correctly, I left a message for you after your comment yesterday, I'm not sure if you can see it. If not I can copy my answer here.

ghiscoding commented 5 months ago

@AlexandrChazov no I don't see your message, maybe you start a Review and you didn't click Finish Review? I often forget to click Finish Review and the comments don't show until it's clicked. But please try what I suggested, I don't see any new commits from your side after my comment, have you tried my suggestion?

AlexandrChazov commented 5 months ago

Yes, I tried it. TypeScript doesn't complain if I don't write any. But anyway it infers DataView.getFilterArgs() as unknown in this case. I even tried to use get/set accessors and it infered unknown type too. If you wish I can don't specify return value of the getter at all.

ghiscoding commented 5 months ago

If you wish I can don't specify return value of the getter at all.

in that case then yes you can do that change, the goal was really to not use any so that it can infer the type if possible

AlexandrChazov commented 5 months ago

maybe you start a Review and you didn't click Finish Review

Thank you a lot, finally I've found this button, it had't been pushed ))

AlexandrChazov commented 5 months ago

you can do that change

done

ghiscoding commented 5 months ago

thanks, I'll push a version in the weekend. Remind me if I forget

AlexandrChazov commented 5 months ago

Don't forget to have a rest )