-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Developed filter and search functionality in the Cases Section #939
Conversation
Will take a closer look in the next few days, video looks nice though! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few comments on the code itself, but here are some other general ones:
- 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. - 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.
- 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.
- 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".
- 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.
Thanks for your work so far, let me know what you think.
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
Yeah, great feature, let me try it.
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.
that is also an good idea.
yeah, that's also sounds good!
Well, all ideas you proposed, it great to implement, I commit over this branch and ask you for suggestions :) |
What about the UI now?
28.02.2024_20.20.21_REC.mp4 |
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 @Julian what do you suggest? |
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". |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
I have changed the description type to |
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? |
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. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Just to summarize slack discussion: |
Yes! it is ready for merging! @Julian (Just mentioning you for the final review and merge) |
@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. |
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. |
Fixes: #37
Description:
Developed filter and search functionality in the Cases Section.
Screenshot(s):
Video Demonstration:
26.02.2024_04.47.57_REC.mp4
📚 Documentation preview 📚: https://bowtie-json-schema--939.org.readthedocs.build/en/939/