ghiscoding / slickgrid-universal

Slickgrid-Universal is a monorepo which includes all Editors, Filters, Extensions, Services related to SlickGrid usage and is also Framework Agnostic
https://ghiscoding.github.io/slickgrid-universal/
MIT License
82 stars 26 forks source link

fix: `filterQueryOverride` provide all search values #1611

Closed zewa666 closed 1 month ago

zewa666 commented 1 month ago

meh there was an issue where suddenly on multi-selects I didn't get the value of the second and onwards selection

stackblitz[bot] commented 1 month ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

zewa666 commented 1 month ago

hmmm do you think its ok to change the signatures for graphql as well @ghiscoding?

ghiscoding commented 1 month ago

oh I see, that was a mistake in the implementation, yes it should be plural since we always deal with a searchTerms array and yes we should update the GraphQL and anywhere else. This feature is relatively new, so it should be ok to update it (with a warning message in the release section) and that seems to be why the build failed in your PR as well

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.8%. Comparing base (e34971d) to head (4ae38b9). Report is 4 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1611 +/- ## ======================================== + Coverage 99.8% 99.8% +0.1% ======================================== Files 198 198 Lines 21795 21796 +1 Branches 7302 7303 +1 ======================================== + Hits 21734 21735 +1 + Misses 61 55 -6 - Partials 0 6 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ghiscoding commented 1 month ago

There's another line to change it seems

Error: src/examples/example10.ts(216,79): error TS2339: Property 'searchValue' does not exist on type 'BackendServiceFilterQueryOverrideArgs'.

zewa666 commented 1 month ago

yeah just saw it ... not sure if I fixed it properly though as the Graphql example doesn't show any results

ghiscoding commented 1 month ago

yeah just saw it ... not sure if I fixed it properly though as the Graphql example doesn't show any results

the GraphQL example only shows the query string (because I don't have a dedicated GraphQL server or fake data like OData), so technically just take a look at the query to see if it work as intended or not. That is also what the E2E tests are testing

ghiscoding commented 1 month ago

looks good, I'll go ahead and merge.

Have you seen my new draft PR #1609? I'm not sure if I'll wait for this PR to be finished before pushing a new release or simply go without it

zewa666 commented 1 month ago

yeah I saw it, didnt have time to properly check it out, plan to do so at home. sounds definitely interesting if only for feature comparison reasons to the big player. I personally feel though that virtual scrolling has far more drawbacks compared to pagination in server-scenarios. nevertheless, I'll give it certainly a try.

next week I'll finally start to combine AI together with the grid and I'm super excited about the potential use cases. the slickgrid project at work made a huge impact so people feel fine to invest a more time 🎉

edit: with regards to new release, no pressure, I've monkey patched my way until the new release is out and whenever that happens I just need to delete those patches.

ghiscoding commented 1 month ago

next week I'll finally start to combine AI together with the grid and I'm super excited about the potential use cases. the slickgrid project at work made a huge impact so people feel fine to invest a more time 🎉

glad to hear this, I rarely hear good stories, I usually hear more about the issues haha

For the release, I think there's enough to go ahead with a new one this weekend and I also want to take my time for that Infinite Scroll feature and it will take time to add tests and also add the feature for GraphQL as well (currently testing with OData since I have test data) and maybe infinite scroll for local dataset (not sure yet)

zewa666 commented 1 month ago

wont say no to that ;)

to give you an idea where new potential features might go with our app (just hacked this together in an hour with a local Mistral LLM and context of the whole row)

grid_ai_yeah

ghiscoding commented 1 month ago

So your last column as a Formatter with a button for the AI, is that it? I haven't much of AI stuff at my work yet but I should dig more into this new tech, it's definitely the future going forward. So is that AI button reading the new row data you've input and provide suggestions or whatever else with that data? It seems quite interesting. It reminds me a bit when I added a translate button for users with different language and they wanted to see it translated on the fly

Side note, my draft PR is also Example 27, so I'm not sure if you wanted to add this new example to my repo but if so, we'll for sure conflict :D

zewa666 commented 1 month ago

dont worry I just took whatever number I saw free. Your example with on the fly translation is actually quite close to it. What one does with it depends on how the prompt is built. In my case I've just instructed it to improve the failure description by using so far text + title.

the interesting part is going to be able to create the prompt along with a template, which allows interpolating the active or other rows to provide context.

I'll play with it next week and let you know more