Bump Docsy to 0.3.x #48721

sftim · 2024-11-14T20:34:46Z

This PR upgrades to a newer version of Docsy: 0.3.0 (preview)

Helps with #32905; also relevant to issue #44002

In the testing I've done, I haven't found any different look or behavior that still needs fixing.

/area web-development

netlify · 2024-11-14T20:43:55Z

✅ Pull request preview available for checking

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`4d01654`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/673ba4b95bcffa0008ca3e49
😎 Deploy Preview	https://deploy-preview-48721--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.

Arhell

/lgtm

k8s-ci-robot · 2024-11-15T00:05:30Z

LGTM label has been added.

Git tree hash: 58a47b9863afa614ac623d9d4a1699b519c970aa

chalin

The Makefile needs the following change, otherwise container-build won't work:

container-build: module-check
	$(CONTAINER_RUN) --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 $(CONTAINER_IMAGE) sh -c "hugo --minify --environment development --destination /tmp/hugo --noBuildLock"

Otherwise, building the container image, and running build and server from the container work (as expected).

sftim · 2024-11-16T12:02:26Z

The Makefile needs the following change, otherwise container-build won't work:
container-build: module-check
	$(CONTAINER_RUN) --read-only --mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 $(CONTAINER_IMAGE) sh -c "hugo --minify --environment development --destination /tmp/hugo --noBuildLock"
Otherwise, building the container image, and running build and server from the container work (as expected).

I think that may be true of the main branch too, so this PR is OK not to try to fix that.

chalin · 2024-11-16T13:51:11Z

Of course. Here you go, an issue and a PR to address the problem:

chalin

This PR introduces and uses new translation keys for i18n_language_name_short_en etc., but these are not a part of Docsy 0.3 nor from the website main branch. I thought that this PR was meant to be only for changes necessary for upgrading to Docsy 0.3, is that correct? If so, could we factor out what appear to be new features unrelated to Docsy 0.3 migration?

sftim · 2024-11-18T20:25:38Z

This PR introduces and uses new translation keys for i18n_language_name_short_en etc., but these are not a part of Docsy 0.3 nor from the website main branch. I thought that this PR was meant to be only for changes necessary for upgrading to Docsy 0.3, is that correct? If so, could we factor out what appear to be new features unrelated to Docsy 0.3 migration?

Sure, I'll get splitting.

chalin

There are also some updates to layouts/partials/head.html that would need to be ported. What would be the best way to share those?

chalin · 2024-11-20T00:32:51Z

hugo.toml

 sidebar_menu_compact = false
+# Show this many levels in compact mode
+ul_show = 3


I'm not sure that setting ul_show makes sense when sidebar_menu_compact = false in line 220 (unless you have custom behavior in layouts/partials/sidebar-tree.html that differs from Docsy's).

I just ran make build-preview with and without this change, and it makes no meaningful difference (there's this strange behavior of heading ID hashes differing by one or two hash characters, but that isn't related to this).

$ git diff | grep ^diff | wc -l 24 $ git diff | grep ^diff | grep -v '_print' | wc -l 6 $ git diff | grep ^diff | grep -v '_print' diff --git a/es/docs/concepts/containers/runtime-class/index.html b/es/docs/concepts/containers/runtime-class/index.html diff --git a/ja/docs/concepts/containers/runtime-class/index.html b/ja/docs/concepts/containers/runtime-class/index.html diff --git a/ko/docs/concepts/containers/runtime-class/index.html b/ko/docs/concepts/containers/runtime-class/index.html diff --git a/pt-br/docs/concepts/containers/runtime-class/index.html b/pt-br/docs/concepts/containers/runtime-class/index.html diff --git a/ru/docs/concepts/containers/runtime-class/index.html b/ru/docs/concepts/containers/runtime-class/index.html diff --git a/zh-cn/docs/concepts/containers/runtime-class/index.html b/zh-cn/docs/concepts/containers/runtime-class/index.html $ git diff -I 'h4 id=' $

So I think that this delta can be dropped, right @sftim?

chalin · 2024-11-20T09:05:46Z

There are also some updates to layouts/partials/head.html that would need to be ported. What would be the best way to share those?

Actually, it seems to be in part of the code that isn't actively used by the website. Do you still want to sync those parts?

Also, in https://kubernetes.slack.com/archives/C1J0BPD2M/p1732054292142139, you posted a question about Docsy 0.6.0. I thought that the plan was to merge these PRs incrementally, starting from this bump to 0.3.0. I'm asking so that I can know where to focus my time as a reviewer on. I'd suggest that we take it one Docsy release at a time. WDYT?

chalin · 2024-11-20T09:34:06Z

/cc @nate-double-u

sftim · 2024-11-20T10:04:38Z

Also, in https://kubernetes.slack.com/archives/C1J0BPD2M/p1732054292142139, you posted a question about Docsy 0.6.0. I thought that the plan was to merge these PRs incrementally, starting from this bump to 0.3.0. I'm asking so that I can know where to focus my time as a reviewer on. I'd suggest that we take it one Docsy release at a time. WDYT?

I would focus on moving to the most recent release that has no major barrier. For example, if 0.6.x is broken but 0.5.x looks good, merge the PR that moves us to 0.5.x

I recommend reviewing #48724

The series of commits will make it easier to revert back to an earlier Docsy version if we need to, plus it helps show how we got there.

sftim · 2024-11-20T10:05:35Z

There are also some updates to layouts/partials/head.html that would need to be ported. What would be the best way to share those?

I don't understand the question @chalin, sorry.

chalin · 2024-11-20T11:31:31Z

layouts/partials/sidebar-tree.html

-{{/* We cache this partial for bigger sites and set the active class client side. */}}
-{{ $sidebarCacheLimit := cond (isset .Site.Params.ui "sidebar_cache_limit") .Site.Params.ui.sidebar_cache_limit 2000 -}}
-{{ $shouldDelayActive := ge (len .Site.Pages) $sidebarCacheLimit -}}
+{{/* Always cache this partial; set the active class client side. */}}
+{{ $shouldDelayActive := true }}


sidebar_cache_limit is already set to 1 in the Hugo config, so effectively shouldDelayActive is true.

Why introduce a difference between the Docsy partial and this K8s override? My hope is that eventually K8s will have as few layout overrides as possible so that it can benefit from easier updates when new Docsy releases come out.

chalin · 2024-11-20T11:32:41Z

There are also some updates to layouts/partials/head.html that would need to be ported. What would be the best way to share those?

I don't understand the question @chalin, sorry.

Here's the diff to layouts/partials/head.html

diff --git a/layouts/partials/head.html b/layouts/partials/head.html
index 97637d10b8..908b7aa00f 100644
--- a/layouts/partials/head.html
+++ b/layouts/partials/head.html
@@ -30,7 +30,10 @@
 {{ end }}
 {{ partialCached "head-css.html" . "asdf" }}
 {{ if and (.Site.Params.offlineSearch) (not .Site.Params.gcs_engine_id) }}
-<script src="https://unpkg.com/[email protected]/lunr.js"></script>
+<script defer
+  src="https://unpkg.com/[email protected]/lunr.min.js"
+  integrity="sha384-203J0SNzyqHby3iU6hzvzltrWi/M41wOP5Gu+BiJMz5nwKykbkUx8Kp7iti0Lpli"
+  crossorigin="anonymous"></script>
 <script src="/js/offline-search.js"></script>
 {{end}}
 {{ partial "hooks/head-end.html" . }}

Do you want to propagate these changes here?

Here's another example. The following was updated in Docsy 0.3.0, and the fix (google/docsy#1070) should be propagated to the K8s layout override like this:

diff --git a/layouts/partials/sidebar-tree.html b/layouts/partials/sidebar-tree.html
index ae6f622df6..86031fbad4 100644
--- a/layouts/partials/sidebar-tree.html
+++ b/layouts/partials/sidebar-tree.html
@@ -43,7 +43,7 @@
   {{ $ulShow := .ulShow -}}
   {{ $currentLang := .currentLang -}}
   {{ $active := and (not $shouldDelayActive) (eq $s $p) -}}
-  {{ $activePath := and (not $shouldDelayActive) ($p.IsDescendant $s) -}}
+  {{ $activePath := and (not $shouldDelayActive) (or (eq $p $s) ($p.IsDescendant $s)) -}}
   {{ $show := cond (or (lt $ulNr $ulShow) $activePath (and (not $shouldDelayActive) (eq $s.Parent $p.Parent)) (and (not $shouldDelayActive) (eq $s.Parent $p)) (not $p.Site.Params.ui.sidebar_menu_compact) (and (not $shouldDelayActive) ($p.IsDescendant $s.Parent))) true false -}}
   {{ $mid := printf "m-%s" ($s.RelPermalink | anchorize) -}}
   {{ $pages_tmp := where (union $s.Pages $s.Sections).ByWeight ".Params.toc_hide" "!=" true -}}

Now this patch doesn't currently impact the generated site files because you've force-set $shouldDelayActive := true, but if ever that changes, the patch made above will be quite a challenge to debug and resolve. WDYT?

sftim · 2024-11-20T11:36:07Z

Responding to #48721 (comment):

I think we can do that work (aligning with Docsy) as a separate PR. No need to minimise the diff in this PR.

@chalin I don't really know Hugo and Docsy well enough to judge what is right. This change seems to work, but if we can help get a better change made: great.
If you or I or SIG Docs more widely can rally help to get things even more polished, that's ideal. But I'd really like to see some change merged. The cost of getting this PR polished is that we languish even longer on a really old Docsy version.

Also, personally, I'd like to be doing less work now rather than more.

chalin

@sftim - thanks for clarifying your priorities in #48721 (comment). To summarize my understanding is that your preference to get Docsy updated ASAP by:

Making minimal changes; the main goal is to not break the site build (nor its rendering)
Accepting the increased technical dept and drift between K8s overrides of Docsy layouts, as some Docsy changes are ignored. Reducing technical debt will be addressed later.

In that case, this PR LGTM. I'd suggest that this get formally approved and merged before we move on to the 0.4.x bump. /cc @nate-double-u, this week's PR wrangler.

/lgtm

--

That being said, I think that it is better practice to avoid introducing design changes (like setting ul_show = 3 and overriding thelayouts/partials/sidebar-tree.html to unconditionally set $shouldDelayActive := true) that are unrelated to the PR title "Bump Docsy to 0.3.x". Managing Docsy updates is complex enough without having to deal with unrelated code updates.

k8s-ci-robot · 2024-11-20T14:32:34Z

@chalin: changing LGTM is restricted to collaborators

In response to this:

@sftim - thanks for clarifying your priorities in #48721 (comment). To summarize my understanding: your preference to get Docsy updated ASAP by making minimal changes; the main goal is to not break the site build (nor its rendering).

In that case, this PR LGTM. I'd suggest that this get formally approved and merged before we move on to the 0.4.x bump. /cc @nate-double-u, this week's PR wrangler.

/lgtm

--

That being said, I think that it is better practice to avoid introducing design changes (like setting ul_show = 3 and overriding thelayouts/partials/sidebar-tree.html to unconditionally set $shouldDelayActive := true) that are unrelated to the PR title "Bump Docsy to 0.3.x". Managing Docsy updates is complex enough without having to deal with unrelated code updates.

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.

sftim · 2024-11-20T16:12:25Z

Either before or after, folks (anyone - not just a Docsy contributor) is welcome to help align more with upstream Docsy, per #48721 (review)

I'll aim to put some time in myself but it might be a while before I get to it.

chalin · 2024-11-20T16:44:43Z

That works for me. I'm eager to see this PR merged, as a first incremental step beyond Docsy 0.2.0.

nate-double-u · 2024-11-20T18:28:23Z

Thanks Tim & Patrice.

reapplying lgtm from @Arhell, and acknowledging Patrice's testing.

/lgtm

This looks good, thanks everyone.

/approve

k8s-ci-robot · 2024-11-20T18:28:31Z

LGTM label has been added.

Git tree hash: c86ecfe236002f156559d37e7f132d7775770e86

k8s-ci-robot · 2024-11-20T18:28:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chalin, nate-double-u

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [nate-double-u]

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

chalin · 2024-11-20T18:32:07Z

Either before or after, folks (anyone - not just a Docsy contributor) is welcome to help align more with upstream Docsy, per #48721 (review)

I'll aim to put some time in myself but it might be a while before I get to it.

@sftim - is there an issue open already that is specifically for this alignment?

sftim · 2024-11-20T18:58:17Z

is there an issue open already that is specifically for this alignment?

#41171 Align with upstream Docsy

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 Nov 14, 2024

sftim mentioned this pull request Nov 14, 2024

Bump Docsy to 0.4.x #48722

Closed

k8s-ci-robot requested a review from mickeyboxell November 14, 2024 20:34

k8s-ci-robot added the language/en Issues or PRs related to English language label Nov 14, 2024

k8s-ci-robot requested a review from tengqm November 14, 2024 20:34

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

sftim mentioned this pull request Nov 14, 2024

Bump Docsy to 0.5.x #48723

Closed

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 14, 2024

This was referenced Nov 14, 2024

Bump Docsy to 0.6.x #48724

Closed

[WIP] Bump Docsy to 0.7.x #48725

Closed

Arhell reviewed Nov 15, 2024

View reviewed changes

k8s-ci-robot assigned Arhell Nov 15, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 15, 2024

chalin reviewed Nov 16, 2024

View reviewed changes

sftim mentioned this pull request Nov 18, 2024

WIP: Bump Docsy to 0.11.x #48726

Closed

sftim force-pushed the 20241031_docsy_zero_three branch from a3c6da9 to 32acb21 Compare November 18, 2024 14:01

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2024

k8s-ci-robot requested a review from Arhell November 18, 2024 14:01

chalin reviewed Nov 18, 2024

View reviewed changes

sftim mentioned this pull request Nov 18, 2024

Localize accessibility text in sidebar nav #48756

Merged

sftim force-pushed the 20241031_docsy_zero_three branch from 32acb21 to 01516c7 Compare November 18, 2024 20:33

Tim Bannister added 2 commits November 18, 2024 20:33

Bump Docsy to v0.3.0

1ff8d96

Adapt sidebar navigation for newer Docsy

4d01654

sftim force-pushed the 20241031_docsy_zero_three branch from 01516c7 to 4d01654 Compare November 18, 2024 20:33

chalin reviewed Nov 20, 2024

View reviewed changes

k8s-ci-robot requested a review from nate-double-u November 20, 2024 09:34

chalin reviewed Nov 20, 2024

View reviewed changes

chalin approved these changes Nov 20, 2024

View reviewed changes

k8s-ci-robot assigned nate-double-u Nov 20, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 20, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 20, 2024

k8s-ci-robot merged commit c4f18b3 into kubernetes:main Nov 20, 2024
5 of 6 checks passed

sftim deleted the 20241031_docsy_zero_three branch November 20, 2024 18:57

This was referenced Nov 22, 2024

Align with upstream Docsy #41171

Open

Upgrade to Docsy 0.5.1 #48807

Closed

[WIP] Make navbar layout vanilla docsy #46409

Closed

chalin mentioned this pull request Dec 10, 2024

24Q1-Q4 planning and activity log for Patrice cncf/techdocs#227

Open

35 tasks

Bump Docsy to 0.3.x #48721

Bump Docsy to 0.3.x #48721

Uh oh!

Conversation

sftim commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull request preview available for checking

Uh oh!

Arhell left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Nov 15, 2024

Uh oh!

chalin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sftim commented Nov 16, 2024

Uh oh!

chalin commented Nov 16, 2024

Uh oh!

chalin left a comment

Choose a reason for hiding this comment

Uh oh!

sftim commented Nov 18, 2024

Uh oh!

chalin left a comment

Choose a reason for hiding this comment

Uh oh!

chalin Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

chalin Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

chalin commented Nov 20, 2024

Uh oh!

chalin commented Nov 20, 2024

Uh oh!

sftim commented Nov 20, 2024

Uh oh!

sftim commented Nov 20, 2024

Uh oh!

chalin Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

chalin commented Nov 20, 2024

Uh oh!

sftim commented Nov 20, 2024

Uh oh!

chalin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Nov 20, 2024

Uh oh!

sftim commented Nov 20, 2024

Uh oh!

chalin commented Nov 20, 2024

Uh oh!

nate-double-u commented Nov 20, 2024

Uh oh!

k8s-ci-robot commented Nov 20, 2024

Uh oh!

k8s-ci-robot commented Nov 20, 2024

Uh oh!

Uh oh!

chalin commented Nov 20, 2024

Uh oh!

sftim commented Nov 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

sftim commented Nov 14, 2024 •

edited

Loading

netlify bot commented Nov 14, 2024 •

edited

Loading

chalin left a comment •

edited

Loading

chalin left a comment •

edited

Loading