bowtie icon indicating copy to clipboard operation
bowtie copied to clipboard

Developed filter and search functionality in the Cases Section

Open Swarnendu0123 opened this issue 1 year ago • 21 comments
trafficstars

Fixes: #37

Description:

Developed filter and search functionality in the Cases Section.

Screenshot(s):

Screenshot (97) Screenshot (98)

Video Demonstration:

https://github.com/bowtie-json-schema/bowtie/assets/94161758/5823aa1f-c4e7-4f0a-b859-02c14f1b8303


📚 Documentation preview 📚: https://bowtie-json-schema--939.org.readthedocs.build/en/939/

Swarnendu0123 avatar Feb 25 '24 23:02 Swarnendu0123

Will take a closer look in the next few days, video looks nice though!

Julian avatar Feb 26 '24 01:02 Julian

  • As you type, the page jumps around. In particualr, when nothing matches (e.g. if you type asdf) the page sort of snaps quite unsatisfyingly, so that the search bar is at the bottom. It'd be a lot better if the position of the search bar didn't change as you type.

okay, so we can have a fixed scrollable container, inside which we can display all the tests, and if the search string is not matching to any tests, we can show no match found or something like that

  • I think it'd be quite nice to have highlighing in some form -- in other words, while you type, it would be nice to highlight the parts of descriptions that are matching your live search in the accordions below.

Yeah, great feature, let me try it.

  • I'm not 100% sure it'd help as I haven't used it, I'd just saved it to look at, but perhaps https://github.com/algolia/autocomplete can help us with this functionality.

Okey, that very great idea, 100% agreed, but I also have not used it before. And also I am not sure that I could implement that or not.

  • It would be nice to be able to search also within the test description (as opposed to the test case description). I'm not sure whether we need a separate box for each, maybe we just always search through both? Then again I think it would be nice to also search through the schema and instance -- and there it does seem like you should be able to turn that on or off. Maybe we should have togglable buttons for "Search in: [ ] schema [ ] instance [ ] test [ ] test case".

that is also an good idea.

  • Taking that yet further, probably it would be nice to have a dropdown which lets you select which implementations you want to see results for. But feel free to leave that for a next PR.

yeah, that's also sounds good!

Thanks for your work so far, let me know what you think.

Well, all ideas you proposed, it great to implement, I commit over this branch and ask you for suggestions :)

Swarnendu0123 avatar Feb 28 '24 13:02 Swarnendu0123

What about the UI now?

  • added highlighter
  • fixed the search box jumping issue

https://github.com/bowtie-json-schema/bowtie/assets/94161758/c665c002-6ad7-4634-b10f-bf2e23a54adb

Swarnendu0123 avatar Feb 28 '24 14:02 Swarnendu0123

It would be nice to be able to search also within the test description (as opposed to the test case description). I'm not sure whether we need a separate box for each, maybe we just always search through both? Then again I think it would be nice to also search through the schema and instance -- and there it does seem like you should be able to turn that on or off. Maybe we should have togglable buttons for "Search in: [ ] schema [ ] instance [ ] test [ ] test case".

I have an idea for enhancing the search functionality. Within the same SearchBox, users could specify the scope of their search. For instance, if someone wants to search within the schema, they could type schema: properties. This way, they can narrow down their search to specific areas such as properties in the schema. We could extend this functionality to other areas as well.

@Julian what do you suggest?

Swarnendu0123 avatar Feb 28 '24 17:02 Swarnendu0123

You're talking about Lucene syntax. That would certainly be nice yeah. Though I don't want to tackle too big of a bite all at once. I'd like to see something fully mergeable in <200 lines of changes, and if need be to add more functionality in follow-ups. So I fully believe making lucene syntax work is doable, and probably there are libraries that will help us do it just like e.g. GitHub search works, but yeah try not to go wild all at once unless you can get it all working well, it's more important that you work reliably -- your first version here was too hasty and didn't work well, so err on the side of "working even if less functionality".

Julian avatar Feb 28 '24 17:02 Julian

I have changed the description type to description: string | JSX.Element. that's why I guess it is breaking the system again and again, let me update the corresponding files.

Swarnendu0123 avatar Feb 28 '24 18:02 Swarnendu0123

That sounds like a very strange change. But also again, work slow and correct rather than haphazardly.

Julian avatar Feb 28 '24 18:02 Julian

That sounds like a very strange change. But also again, work slow and correct rather than haphazardly.

Can you please build my final commit locally and give me some suggestions, so that I can get an overview how to proceed?

Swarnendu0123 avatar Feb 28 '24 18:02 Swarnendu0123

Politely no. It's been less than an hour since we had a design discussion, and four hours total since we first discussed. There's no way that's enough time spent on your part just making sure you have as much worked out as you can before asking specific questions. Being direct, it makes it seem even more like laziness -- I've tried to subtly give you this feedback so I'll try less subtle now :). When I say "work more carefully" this is what I mean. Don't throw something poorly thought out over to me for me to work out the problems with. It's ok to get stuck, it's not ok to spend minimal time and then kick it over.

Julian avatar Feb 28 '24 18:02 Julian

Hello @Julian! please review my changes and give me some feedback.

Swarnendu0123 avatar Mar 03 '24 08:03 Swarnendu0123

Apologies for the delay!

I have looked at Algolia's docs, but I leave it for future to implement. It would be great if we can create a separate issue for it.

I'm not 100% sure it'd help as I haven't used it, I'd just saved it to look at, but perhaps https://github.com/algolia/autocomplete can help us with this functionality.

Also this, I have left for future, because you already mentioned, you are not expecting >200 lines of changes. It would be great if you create a separate issue for that.

Following your suggestions, here are the updates:

  • Implemented filter functionality using checkboxes.
  • Removed the "all" option from the filter for simplicity.
  • Ensured consistency by using Regular Expressions in both instances.

You can review the final outcome here:

https://github.com/bowtie-json-schema/bowtie/assets/94161758/8d87d49d-74d4-445c-bd4e-704795a1c5fc

Swarnendu0123 avatar Mar 05 '24 06:03 Swarnendu0123

Thanks for the review @harrel56 !

using 3rd party library - suggested lib has React variant (https://www.npmjs.com/package/react-instantsearch-core) but it looks kind of big for what we are trying achieve here, still maybe it's the best solution. I had good experience with mui-base but it's still in beta and you probably would still have to implement all the filtering and highlighting - but I love it for its flexible API

Yes! We can probablly add that point into https://github.com/json-schema-org/community/issues/609, as we are redesigning the whole section in this project.

filtering by not just a test description - schema and instance data would have to be converted to string (from json). But even now it feels kind of laggy so I don't know how it would be like. BTW probably there can be just a one regex test - maybe that would help a little.

That's actually a great idea! and I actually suggested Lucene Syntax for this section. for example,

schema:"something" AND instance:"something"

this will make much easier the search. Also it can we include in the https://github.com/json-schema-org/community/issues/609.

what are your views on it? @harrel56 @Julian.

Swarnendu0123 avatar Mar 07 '24 17:03 Swarnendu0123

Hey @Swarnendu0123, I'm gonna be out for like a 2 weeks so probably won't be able to respond. Hopefully, you @Julian could check out my comments and pick up from there if needed, thanks!

harrel56 avatar Mar 07 '24 18:03 harrel56

I have noticed something odd here, on selecting successful we are getting the whole array! And it would be nice, if we can get how many cases are there in the final array.

https://github.com/bowtie-json-schema/bowtie/assets/94161758/04b83208-6a2d-4bfb-8983-ba5df4a52a01

let me push one commit, so that you can check it.

Swarnendu0123 avatar Mar 07 '24 19:03 Swarnendu0123

@Swarnendu0123 is this ready for review you think?

Julian avatar Mar 18 '24 16:03 Julian

@Swarnendu0123 is this ready for review you think?

I have implemented almost all the features which I could. Some suggestions by @harrel56 are still left. But we will improve it eventually through future PRs. So yes! it is ready for the review.

Swarnendu0123 avatar Mar 18 '24 16:03 Swarnendu0123

Just to summarize slack discussion: The only blocking issue is https://github.com/bowtie-json-schema/bowtie/pull/939#discussion_r1513542530, if it's sorted out and the 2 mentioned cases pass, I think we can finally merge it. Also the lib for highlighting (https://www.npmjs.com/package/react-highlight-words) looks fine :)

harrel56 avatar Apr 17 '24 19:04 harrel56

Just to summarize slack discussion: The only blocking issue is #939 (comment), if it's sorted out and the 2 mentioned cases pass, I think we can finally merge it. Also the lib for highlighting (https://www.npmjs.com/package/react-highlight-words) looks fine :)

Yes! it is ready for merging! @Julian (Just mentioning you for the final review and merge)

Swarnendu0123 avatar Apr 19 '24 17:04 Swarnendu0123

@Swarnendu0123 Is that blocking issue resolved then?

Julian avatar Apr 21 '24 07:04 Julian

@Swarnendu0123 Is that blocking issue resolved then?

No, this the issue needs https://www.npmjs.com/package/react-highlight-words, this PR is already have changes ~190 lines. It will be hard to review if I implement it in the same PR. I will implement the lib in subsequent PR(s). We can merge it for now.

Swarnendu0123 avatar Apr 21 '24 07:04 Swarnendu0123

The word "blocking" means "this isn't usable without it" :) so that doesn't sound right, but I'll have a look myself at what state this is in at this point.

Julian avatar Apr 21 '24 07:04 Julian