-
Notifications
You must be signed in to change notification settings - Fork 380
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
Search through https://search.quarkus.io #1825
Conversation
🙈 The PR is closed and the preview is expired. |
9bae26e
to
5ca41f8
Compare
If this supports localized sites (es.quarkus.io, pt.quarkus.io, cn.quarkus.io, and ja.quarkus.io), it's really appriciated :) |
From what I saw the guides in localized sites are still in English. And I don't think there are plans to translate the guides. So, since this search is about the content of the guides, I don't think there's anything to do... |
Not sure about other locales, but speaking of ja.quarkus.io, which I'm working on, all the v2.7 guides are localized (https://ja.quarkus.io/version/2.7/guides/), and more than half of the latest v3.5 guides are localized(https://ja.quarkus.io/guides/). |
That's definitely different from the other locales, but sure we can index that as well. Could you please open an issue at https://github.com/quarkusio/search.quarkus.io with some information on where to find these guides (GitHub repo, path in the repo in particular). Also I have zero experience with search in Japanese and can't even read it, so it's definitely going to be challenging to tune it for good relevance; I'll need feedback from someone who can understand the results. |
Of course I'm happy to help this :) I filed an issue: quarkusio/search.quarkus.io#33 |
2207750
to
a7958da
Compare
I tried a few queries but didn't spot much difference in list of matches - wondering if i'm just choosing bad ones that are already tagged :) Got some good examples? One thing I did notice is that search no longer retain tutorials/concepts/references section titles - is that intentional? The order still seem to be the exact same as old one thus not sure ? |
627eedb
to
a17dced
Compare
I think the timeout I originally set to fall back to Javascript search was too low; search is fast on the server side but our Openshift cluster seems to be located far from Europe and this results in rather high latency depending on your connection. So, in short, you've been testing the fallback to Javascript search. Which doesn't yield good search results indeed, but at least fallback works, so, yay! I pushed an update with a higher timeout. Try again with Before: After: Another search you can try is
It's intentional: we need to remove section titles in order to provide relevance sort, and yes we do sort by relevance. The order bieng the same as the old one is a result of using Javascript search, see above. It just happens that I remove section titles with Javascript search as well, because I didn't want to duplicate the code just for this. |
3b241ac
to
c3cc7cb
Compare
Hey @edewit @ia3andy , I've been told you know your way around Javascript :) When you find some time, would you be so kind as to review this? It's probably never going to be perfect, but I'd like to at least avoid obvious mistakes... I added instructions on how to run it locally in the PR description, and you can test it live here: https://quarkus-website-pr-1825-preview.surge.sh/guides/ @brunobat reported one bug already: it seems that if you type relatively fast in the text box, characters get mangled. I think it comes from the fact the text box gets refreshed while you type. I don't know why it even gets refreshed since, well, it's supposed to be write-only, but maybe you have an idea? |
I recorded a video to demo the effect: |
c3cc7cb
to
55e59a6
Compare
I played a bit more with it today and given the search is using a AND when fallabacking to Javascript, I would lean towards also using a AND when using the service. |
|
FWIW the search service now indexes the full HTML body of each guide, which includes e.g. configuration property references. So the search now matches on configuration properties. |
My only concern is that we already use Lit and React in other Quarkus projects (code.quarkus and DevUI) using yet another framework will make things hard to maintain in the future. Would it be possible to switch to LitElement? |
If someone sends a PR, sure. Personally I've already spent a few days just making it work, so I can't afford to spend a week learning another framework and trying to make it work... |
From what I've looked, there is not much UI code yet, so better choose wisely. Also if you look at Lit, it seems really close to the code you provided (I am not familiar with Vue either) so I don't think it will be a big gap from one to the other... |
That's related to my concern. When I say I'm not good at Javascript, I mean it. This "not much" literally took me days of back and forth to get something that seems to work (with that one remaining bug...). In fact I'm pretty sure I've spent as much time on the frontend than on the backend at this point... If we exclude OpenShift deployment :D
And there you have it: "not much UI code" => a few days; "not a big gap" => how many more days? I really can't afford to spend that kind of time right now, especially since this is essentially a side project and I'm late on pretty much every main project I'm assigned to (Hibernate Search, Quarkus Hibernate extensions, ...). If you feel strongly about this I'd gladly accept a patch from an expert like you that basically erases my stuff with the same features (including infinite scrolling etc.) but on a better framework and with better code. |
1680b01
to
6f031c0
Compare
6f031c0
to
4a9c19d
Compare
97685fb
to
fae2774
Compare
I pushed an update this morning to use the new domain ( I'll try pushing again. |
fae2774
to
7828b01
Compare
7828b01
to
8996fa5
Compare
6d48c7d
to
e3fc194
Compare
We have just agreed to merge this early next year: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/search.2Equarkus.2Eio/near/409365021 |
e3fc194
to
234f821
Compare
tried finding info about "quarkus.websocket.max-frame-size" but couldn't get it to show something relevant. tried the full string with and without quotes - also tried "max-frame-size" and even with quotes it kept searching for individual parts. Should have found at least https://quarkus.io/guides/all-config that has it mentioned. |
FWIW this is being discussed there: quarkusio/search.quarkus.io#117 |
Fixed and deployed just now. Try it out :) |
234f821
to
366d1f6
Compare
Turns out latency is quite bad on the production server for search.quarkus.io.
366d1f6
to
b39a5a7
Compare
Merged! Thanks all for your comments. If you notice any problems, please report to https://github.com/quarkusio/search.quarkus.io/issues |
Creating as draft, for discussion. Please do not merge yet.
Summary of the changes:
/guides
and/version/*/guides
is now implemented with a Vue.js application, replacing the custom Javascript ofsearch-filter.js
.To test locally:
quarkus dev
)./serve.sh
ordocker compose up
)You should notice search results are slightly more relevant, though we didn't try to tune search very much for now. You will see improvements though, e.g. matches against content that is not in the title/summary but elsewhere in the guide