Revise search to align better with Docsy #47975

sftim · 2024-09-17T20:24:52Z

This is a small bit of change that (only slightly) helps us adopt Docsy. The associated JavaScript files are all moved to assets, allowing build-time minification.

Preview, preview

/area web-development

sftim · 2024-09-17T20:25:19Z

assets/js/search-google.js

Moved, not new.

sftim · 2024-09-17T20:25:31Z

static/js/search.js

Moved, not removed.

netlify · 2024-09-17T20:34:11Z

✅ Pull request preview available for checking

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`38cec03`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67abdcfb8b10860008fc6161
😎 Deploy Preview	https://deploy-preview-47975--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dipesh-rawat

It looks like the search is now visible in the navbar on my smaller viewport (not visible on the current production website), even though I can see the intent to hide it using a media query in CSS (here). Not sure if this is intended behaviour or possibly a regression with these changes? Otherwise, these proposed changes are good.

sftim · 2024-09-24T08:12:46Z

Hmm, the style change you mention @dipesh-rawat doesn't match intent. I wonder if there's a tool to help uncover this kind of impact and test if it's a problem?

I don't have a fleet of browsers etc to try it out with.

sftim · 2024-10-19T14:07:58Z

@dipesh-rawat PTAL; I changed the approach here.

sftim · 2024-10-19T14:08:20Z

@divya-mohan0209 if you have time to add your feedback, I'd appreciate that too.

sftim · 2024-10-19T14:08:56Z

layouts/shortcodes/site-searchbar.html

+{{- partial "search-form.html" . }}
+</section>


Making this a section by itself allows us to make the search form take up less than the full viewport width.

sftim · 2024-10-19T14:09:38Z

layouts/_default/search.html

+        <section class="row td-search-query">
          <div class="col-12 col-md-4 offset-md-2">
-            <form class="td-sidebar__search d-flex align-items-center">
-              {{ partial "search-input.html" . }}
-            </form>
+            {{- partial "search-form.html" . }}
          </div>
+        </section>


The search results page also lets you do a new query. I've split that into its own section.

k8s-ci-robot · 2025-02-03T22:07:36Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from sftim. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sftim · 2025-02-03T22:48:20Z

Will look wrong until #49630 has merged.

sftim · 2025-02-03T22:49:16Z

assets/js/search-google.js

+          }
+
+          var query = $(this).val();
+          var searchPage = $(this).closest('form').data('search-page') + "?q=" + query;


Changed from the original line:

- var searchPage = $(this).data('search-page') + "?q=" + query; + var searchPage = $(this).closest('form').data('search-page') + "?q=" + query;

sftim · 2025-02-03T22:49:30Z

assets/js/search-google.js

Compare with https://github.com/kubernetes/website/blob/8a97a3cafd0d6f115987c64940e102c6f49af044/assets/js/search.js

sftim · 2025-02-04T09:57:31Z

Hmm. The mobile view doesn't seem right.

shurup · 2025-02-04T11:12:48Z

@sftim why? It seems to be fine (with an exception for the 90% width, which would be fixed by your PR).

P.S. Personally, I would also remove unnecessary left/right paddings to leave more space for the content itself. (If that sounds good, I can suggest a fix for it here.)

sftim · 2025-02-04T11:16:15Z

Maybe it's just an artefact of the width. I'll octopus them together and check.

sftim · 2025-02-10T23:47:47Z

assets/js/search.js

-          var searchPage = $(this).data('search-page') + "?q=" + query;
-          document.location = searchPage;
+    window.renderGoogleSearchResults = () => {
+        var cx = '013288817511911618469:elfqqbqldzg';


Future work: move this to a site variable or a build variable.

sftim · 2025-02-11T11:52:29Z

Needs more work; the search doesn't actually work right now!

Co-authored-by: Dmitry Shurupov <[email protected]>

sftim · 2025-02-23T23:12:34Z

If #49724 does a better job: great! This one can close.

k8s-ci-robot added area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 17, 2024

k8s-ci-robot requested a review from divya-mohan0209 September 17, 2024 20:25

k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Sep 17, 2024

k8s-ci-robot requested a review from salaxander September 17, 2024 20:25

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 17, 2024

sftim commented Sep 17, 2024

View reviewed changes

assets/js/search-google.js

Copy link

Contributor Author

sftim Sep 17, 2024

Choose a reason for hiding this comment

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

Moved, not new.

sftim commented Sep 17, 2024

View reviewed changes

static/js/search.js Outdated

Copy link

Contributor Author

sftim Sep 17, 2024

Choose a reason for hiding this comment

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

Moved, not removed.

sftim marked this pull request as draft September 17, 2024 20:26

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 17, 2024

sftim force-pushed the 20240917_revise_search branch from fb48337 to 81868af Compare September 21, 2024 16:32

sftim marked this pull request as ready for review September 21, 2024 16:32

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 21, 2024

k8s-ci-robot requested review from drewhagen and katcosgrove September 21, 2024 16:32

dipesh-rawat reviewed Sep 24, 2024

View reviewed changes

sftim marked this pull request as draft October 9, 2024 02:50

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 9, 2024

sftim force-pushed the 20240917_revise_search branch 2 times, most recently from 34a98cd to bc1c158 Compare October 19, 2024 02:10

sftim marked this pull request as ready for review October 19, 2024 02:11

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 19, 2024

k8s-ci-robot requested review from reylejano and tengqm October 19, 2024 02:11

sftim commented Oct 19, 2024

View reviewed changes

sftim marked this pull request as draft December 31, 2024 17:10

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 31, 2024

sftim force-pushed the 20240917_revise_search branch from ab2abec to bf9f537 Compare February 3, 2025 22:07

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2025

sftim force-pushed the 20240917_revise_search branch from bf9f537 to bcae880 Compare February 3, 2025 22:21

sftim marked this pull request as ready for review February 3, 2025 22:21

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 3, 2025

k8s-ci-robot requested a review from nate-double-u February 3, 2025 22:21

sftim force-pushed the 20240917_revise_search branch from bcae880 to bfcdf6d Compare February 3, 2025 22:47

sftim commented Feb 3, 2025

View reviewed changes

sftim force-pushed the 20240917_revise_search branch from bfcdf6d to 742923e Compare February 3, 2025 22:51

SayakMukhopadhyay mentioned this pull request Feb 4, 2025

Fix incorrect width for search page #49630

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2025

sftim force-pushed the 20240917_revise_search branch from 742923e to 91b0930 Compare February 10, 2025 23:47

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2025

sftim commented Feb 10, 2025

View reviewed changes

sftim marked this pull request as draft February 11, 2025 11:52

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 11, 2025

Revise search to align better with Docsy

35e73aa

Co-authored-by: Dmitry Shurupov <[email protected]>

sftim force-pushed the 20240917_revise_search branch from 91b0930 to 35e73aa Compare February 11, 2025 19:41

WIP

38cec03

SayakMukhopadhyay mentioned this pull request Feb 12, 2025

Rework the search scripts to make it closer to docsy #49724

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise search to align better with Docsy #47975

Revise search to align better with Docsy #47975

sftim commented Sep 17, 2024 •

edited

Loading

sftim Sep 17, 2024

sftim Sep 17, 2024

netlify bot commented Sep 17, 2024 •

edited

Loading

dipesh-rawat left a comment

sftim commented Sep 24, 2024

sftim commented Oct 19, 2024

sftim commented Oct 19, 2024

sftim Oct 19, 2024

sftim Oct 19, 2024

k8s-ci-robot commented Feb 3, 2025

sftim commented Feb 3, 2025

sftim Feb 3, 2025

sftim Feb 3, 2025 •

edited

Loading

sftim commented Feb 4, 2025

shurup commented Feb 4, 2025

sftim commented Feb 4, 2025

sftim Feb 10, 2025

sftim commented Feb 11, 2025

sftim commented Feb 23, 2025

Revise search to align better with Docsy #47975

Are you sure you want to change the base?

Revise search to align better with Docsy #47975

Conversation

sftim commented Sep 17, 2024 • edited Loading

sftim Sep 17, 2024

Choose a reason for hiding this comment

sftim Sep 17, 2024

Choose a reason for hiding this comment

netlify bot commented Sep 17, 2024 • edited Loading

✅ Pull request preview available for checking

dipesh-rawat left a comment

Choose a reason for hiding this comment

sftim commented Sep 24, 2024

sftim commented Oct 19, 2024

sftim commented Oct 19, 2024

sftim Oct 19, 2024

Choose a reason for hiding this comment

sftim Oct 19, 2024

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 3, 2025

sftim commented Feb 3, 2025

sftim Feb 3, 2025

Choose a reason for hiding this comment

sftim Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

sftim commented Feb 4, 2025

shurup commented Feb 4, 2025

sftim commented Feb 4, 2025

sftim Feb 10, 2025

Choose a reason for hiding this comment

sftim commented Feb 11, 2025

sftim commented Feb 23, 2025

sftim commented Sep 17, 2024 •

edited

Loading

netlify bot commented Sep 17, 2024 •

edited

Loading

sftim Feb 3, 2025 •

edited

Loading