Skip to content

Add DocSearch v3 Integration #16394

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

Add DocSearch v3 Integration #16394

wants to merge 0 commits into from

Conversation

AdeshGhadage
Copy link
Contributor

Description

integrated algolia for site search

Reviewers

  • Ambient
  • Docs
  • Installation
  • Networking
  • Performance and Scalability
  • Extensions and Telemetry
  • Security
  • Test and Release
  • User Experience
  • Developer Infrastructure
  • Localization/Translation

@AdeshGhadage AdeshGhadage requested a review from a team as a code owner April 8, 2025 11:51
@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test labels Apr 8, 2025
@istio-testing
Copy link
Contributor

Hi @AdeshGhadage. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Arhell
Arhell reviewed Apr 9, 2025
View reviewed changes
Copy link
Member

@Arhell Arhell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Apr 9, 2025
@craigbox
Copy link
Contributor

craigbox commented Apr 9, 2025

You're about 10 steps ahead here, @AdeshGhadage!

  1. results are not yet scoped to versions
Screenshot 2025-04-09 at 4 33 46 PM
  1. The default UI looks out of place on the page; we should retain the single magnifying glass in the first instance.

@craigbox craigbox added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Apr 9, 2025
@AdeshGhadage
Copy link
Contributor Author

when i test docsearch implmentation locally it was not showing any results. i didn't get whats the issue i want to test it in Deploy Preview

@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR needs to be rebased before being merged size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 24, 2025
@craigbox
Copy link
Contributor

Nice!

I think we need to implement facets on the results page next. All I see is the same page repeated for every indexed version. Ultimately I want to see (a) only latest (b) drop-downs letting you pick a version and a language to refine your search (c) optionally, have the search pre-fill the version/language facet if you're on an older version or a different language.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label May 28, 2025
@istio-testing
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

craigbox
craigbox reviewed May 29, 2025
View reviewed changes
layouts/_default/baseof.html Outdated
@@ -183,5 +183,6 @@
{{ partial "icon.html" "top" }}
</button>
</div>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

assets/sass/misc/_header.scss Outdated
@@ -511,4 +511,4 @@
}
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linter may complain about no blank line?
otherwise this is a Unix vs Windows line ending issue, you can see it in a few other files

data/versions.yml Outdated
Comment on lines 2 to 5
main: "1.25"

# The version of Istio currently documented in preliminary.istio.io
preliminary: "1.26"
preliminary: "1.27"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably rebase this PR against the current master. Let me know if you need help doing that.

data/versions.yml Outdated
Comment on lines 7 to 14
supported_versions:
- latest
- v1.25
- v1.24
- v1.23
- v1.22
- v1.21
- v1.20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(a) we shouldn't need to list all these
(b) we only want to search N-5; so for 1.26, that would be 1.21 and up

layouts/_default/search.html Outdated
Comment on lines 51 to 54
function extractVersionAndLangFromPath() {
const pathParts = window.location.pathname.split('/');
let version = 'latest';
let lang = 'en';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you write this code, or is it based on something from Algolia?
Should it be tested? Should it be in its own JS/TS file?

layouts/_default/search.html Outdated
Comment on lines 151 to 153
<style>
.search-results {
padding: 1rem;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note, will need to be in its own SCSS file soon enough

@AdeshGhadage AdeshGhadage force-pushed the feature/docsearch-v3 branch from 9ba5438 to 606099a Compare May 29, 2025 07:39
@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@craigbox craigbox craigbox left review comments

@Arhell Arhell Arhell left review comments

Assignees
No one assigned
Labels
area/user experience do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. kind/docs needs-rebase Indicates a PR needs to be rebased before being merged ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AdeshGhadage @istio-testing @craigbox @Arhell @istio-policy-bot