-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add search functionality #189
Conversation
Pulled a clone of this and it is a nice addition. I do wonder if it would be better to add the search box to the top menu bar instead of it being a page of its own in the side nav. That way it can be accessed from anywhere instead of having to go to the search page first. |
Things to consider/discuss before moving on:
~(Obviously unit tests are currently not complete) ~ Any thoughts? |
A possible alternative to lunr could be fuse.js A quick look at npmtrends shows that it is the most active of any of the lightweight js search engines. |
d5e995a
to
6d675f8
Compare
d5ef5c6
to
77cac60
Compare
This is based on Lunrjs. We assemble documents when generating the site, then Lunr builds an index on the client on page load and we search through that.
@dirkgroot I'm pretty happy now how it works (and looks). Feel free to give it a try. I'm looking forward to your impressions and feedback. |
src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SearchPage.kt
Show resolved
Hide resolved
src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SearchPage.kt
Show resolved
Hide resolved
src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/indexing/SoftwareSystemContext.kt
Show resolved
Hide resolved
src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SearchPage.kt
Show resolved
Hide resolved
I like it. It looks nice, and I it's very useful. I do, however, think that with changes like this, we need to be wary of getting to a point where the code required for the "richer" user experience is getting too hard to maintain. This tool was never designed with a "rich" user experience in mind, so any JavaScript code added to this will never be a "first class citizen". Nothing is set up for unit testing, or keeping dependencies up-to-date. Adding more stuff like this, will increase the risk of things breaking when changes are made. This is part of the reason why I'm not really keen on merging #180. However, functionality-wise, I do think that this is a really useful addition. |
src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/PageHeader.kt
Outdated
Show resolved
Hide resolved
yeah, I'm seeing this too. As discussed a while ago, for the long term some real end-to-end tests are needed to mitigate the risks you've mentioned and we should think about how renovate can catch up on client package updates. |
Still work-in-progress, but works already.