fix: next/prev navigation in docs #2871

fykaa · 2024-10-29T17:58:31Z

So, it is a quite strange distinction.

I looked into it more, but I could not find any notable differences between Kargo and Akuity that could lead to a navigation failure in the live document.

Aside from that, I did observe that some versions of Package.json were not updated.

But since the navigation appear well locally, I am not sure how to test this change in the live document to see if updating the version was able to improve things.

cc: @Marvin9 ptal

netlify · 2024-10-29T17:58:48Z

✅ Deploy Preview for docs-kargo-io ready!

Name	Link
🔨 Latest commit	`0e3807e`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-kargo-io/deploys/6728b129ecaf1c0008567adc
😎 Deploy Preview	https://deploy-preview-2871.docs.kargo.io
📱 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.

codecov · 2024-10-29T18:03:27Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.98%. Comparing base (8d43018) to head (0e3807e).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2871   +/-   ##
=======================================
  Coverage   49.98%   49.98%           
=======================================
  Files         275      275           
  Lines       24526    24526           
=======================================
  Hits        12259    12259           
  Misses      11596    11596           
  Partials      671      671

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

krancour · 2024-10-29T18:17:26Z

I am not sure how to test this change in the live document

There is a link for a preview of any documentation modifications in the checks section at the bottom of every PR.

In this case, you will see that the doc build actually fails with these changes.

I don't believe this PR will solve the issue.

What I would suggest is using your browser's tools to attempt identifying a font or any other resource that isn't loading correctly on the live site, which may explain this. It could be that some font has been excluded from the static production build that is included when running locally from a Docusaurus server.

Marvin9 · 2024-10-29T18:44:25Z

Took a glance. I suspect its related to encoding CSS files that docusaurus generates.

One of this or any other reason could be problem

character is converted to "?" in css on build time (by css minifier)
when browser requests css file and receives it and transform some characters to "?"
netlify transforms it based on request headers and gives browser with characters "?"
Couldn't think of other reason but there might be

I would start looking at build css files that netlify is serving (if I could download) and search for ".pagination-nav__link--next .pagination-nav__label:after" style

krancour · 2024-10-29T19:29:46Z

Here's a new datapoint:

FROM node:latest AS builder

COPY ./docs /docs

WORKDIR /docs

RUN npm install pnpm -g && pnpm install && pnpm build

FROM nginx:alpine

COPY --from=builder /docs/build /usr/share/nginx/html

EXPOSE 80

CMD ["nginx", "-g", "daemon off;"]

docker build -t site .
docker run -it -p 8080:80 site

Everything works.

So everything comes out ok when:

Running locally
Building the static site and serving it from Nginx

The issue seems then to only happen with Netlify.

I've noticed warnings in the build logs in the past. Maybe they are worth looking into.

fykaa · 2024-10-29T21:15:52Z

I would start looking at build css files that netlify is serving (if I could download) and search for ".pagination-nav__link--next .pagination-nav__label:after" style

that suspicion about encoding issues is spot on.
something in the build or deployment process is causing character encoding problems.

.pagination-nav__link--prev .pagination-nav__label::before {
      content: '� ';
    }

.pagination-nav__link--next .pagination-nav__label::after {
      content: ' �';
    }

So everything comes out ok when:
Running locally
Building the static site and serving it from Nginx

i see, so this strengthens the case that the issue is specific to how Netlify handles the build particularly with character encoding in the css

since i dont have access to logs on netlify’s dashboard through gh actions, i am not aware of any warnings that might occur during the css minification. I will try an explicit definition of pagination to bypass encoding transformations netlify might apply

fykaa · 2024-10-29T21:27:13Z

This commit solves the problem with netlify, but there could be a more optimized approach to preempt similar encoding issues in the future—perhaps through a different font choice or other encoding settings.

fykaa · 2024-10-29T21:28:22Z

docs/docusaurus.config.js

+      docs: {
+        sidebar: {
+          hideable: true,
+        },
+      },


This is a different feature and not part of the Issue.

I'd rather not include unrelated changes in a bug fix. Just make a separate PR for this please. This change is actually easier to say yes to on its own that this PR is, so it moves that change along faster.

fykaa · 2024-10-29T21:30:27Z

docs/package.json

@@ -17,11 +17,11 @@
  },
  "packageManager": "pnpm@9.0.3",
  "dependencies": {
-    "@cmfcmf/docusaurus-search-local": "^0.11.0",
+    "@cmfcmf/docusaurus-search-local": "^1.1.0",


These version updates aren’t necessary to resolve this issue, but in my opinion, it's better to use the latest versions as long as they don't disrupt anything on our site at the moment

Upgrading major versions of a dependency usually runs a higher risk of a compatibility issue. I would suggest doing this as a separate PR.

krancour · 2024-10-29T23:36:09Z

docs/src/css/custom.css

+
+/* Pagination symbols for next and previous buttons */
+.pagination-nav__link--prev .pagination-nav__label:before {
+  content: '« ';
+}
+
+.pagination-nav__link--next .pagination-nav__label:after {
+  content: ' »';
+}


Do you know why this fixes the problem?

because custom styles tend to stay unminified in netlify's handling of docusaurus files

I actually did dig docs.akuity.io CNAME and figured akuity docs site isn't served by Netlify.
Hence, it perhaps were a netlify issue for docs.kargo.io

as Mayur mentioned it seems to come from an encoding conflict during the CSS minification or request processing

docusaurus’s CSS originally defined the content values for pagination symbols correctly, but netlify serves these characters as �

and i think by explicitly setting the content property for .pagination-nav__link within a custom css file, we avoid re-encoding during minification.

But imo there is more to this issue. It occurs to me that unless I don't use upgraded versions for docusaurus-search-local, clsx and typescript, the site will still not build those symbols correctly.

A lot of that isn't adding up though...

Content type headers on the CSS correctly reflect UTF-8 encoding.

A UTF-8 encoded file, would have no difficulty displaying », so if you open the downloaded and see the � character, that actual character is what's in the file.

So the question is how it got there.

If you build the site locally or in a container, you'll find » in the minified CSS; not �. This means that the minification process, and more broadly the build process, isn't the problem in and of itself.

The only possibilities I see at the moment are:

Build/minification is producing a different result in the Netlify build environment for a reason we have not yet identified.

If we can nail this down as being the issue, we can probably fix it because we have a reasonable degree of control over that build environment.

The build/minification, even in the Netlfiy build environment, is working perfectly and the file is being corrupted somehow after that. The file would be served from the Netlify CDN, which calls that into question.

It occurs to me that unless I don't use upgraded versions for docusaurus-search-local, clsx and typescript, the site will still not build those symbols correctly.

If this were the issue, we'd be able to replicate the problem in a local build.

Marvin9 · 2024-10-31T17:07:05Z

facebook/docusaurus#10465

krancour · 2024-10-31T20:15:30Z

I can force Netlify's build env to use a different version of Node. Will try tomorrow.

fykaa · 2024-11-04T11:30:28Z

I can force Netlify's build env to use a different version of Node.

Since we are already using a netlify.toml file, I tried changing

[build.environment]
NODE_VERSION = "22.11.0"

Also, I do not have access to the Netlify Build logs it seems

Signed-off-by: Faeka Ansari <faeka6@gmail.com>

krancour · 2024-11-04T23:08:47Z

I can force Netlify's build env to use a different version of Node.

Well, I clearly never got around to that as planned. Thank you for beating me to it, @fykaa. 😄

I can see in the logs that the build for the doc preview of this PR used Node 22.11.0. You should also be able to see in the preview that the characters in the prev/next buttons are displaying correctly.

Good teamwork. Thanks @Marvin9 for digging up that relevant issue from the docusaurus repo. @fykaa thanks for the fix.

LGTM

Signed-off-by: Faeka Ansari <faeka6@gmail.com> (cherry picked from commit 7813eff)

akuitybot · 2024-11-04T23:20:46Z

Successfully created backport PR for release-1.0:

chore(backport release-1.0): fix: next/prev navigation in docs #2898

fykaa self-assigned this Oct 29, 2024

fykaa requested review from a team as code owners October 29, 2024 17:58

fykaa force-pushed the fix-ui-navigation branch 2 times, most recently from 7f747ec to 0cad23e Compare October 29, 2024 21:19

fykaa commented Oct 29, 2024

View reviewed changes

krancour reviewed Oct 29, 2024

View reviewed changes

fykaa force-pushed the fix-ui-navigation branch from 0cad23e to d46199e Compare October 30, 2024 13:58

fykaa marked this pull request as draft October 30, 2024 15:15

fykaa force-pushed the fix-ui-navigation branch 2 times, most recently from 7516592 to b11c313 Compare November 4, 2024 11:12

update version

0e3807e

Signed-off-by: Faeka Ansari <faeka6@gmail.com>

fykaa force-pushed the fix-ui-navigation branch from 9ed7eae to 0e3807e Compare November 4, 2024 11:33

fykaa marked this pull request as ready for review November 4, 2024 11:48

krancour added this to the Live Docs milestone Nov 4, 2024

krancour added area/documentation kind/bug priority/normal labels Nov 4, 2024

krancour added the backport/release-1.0 label Nov 4, 2024

krancour self-requested a review November 4, 2024 23:09

krancour approved these changes Nov 4, 2024

View reviewed changes

krancour added this pull request to the merge queue Nov 4, 2024

Merged via the queue into akuity:main with commit 7813eff Nov 4, 2024
25 checks passed

github-actions bot pushed a commit that referenced this pull request Nov 4, 2024

fix: next/prev navigation in docs (#2871)

c6601b4

Signed-off-by: Faeka Ansari <faeka6@gmail.com> (cherry picked from commit 7813eff)

akuitybot mentioned this pull request Nov 4, 2024

chore(backport release-1.0): fix: next/prev navigation in docs #2898

Merged

krancour mentioned this pull request Nov 8, 2024

docs: previous/next links at bottom of documents have unresolved characters #2870

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: next/prev navigation in docs #2871

fix: next/prev navigation in docs #2871

fykaa commented Oct 29, 2024

netlify bot commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading

krancour commented Oct 29, 2024

Marvin9 commented Oct 29, 2024 •

edited

Loading

krancour commented Oct 29, 2024 •

edited

Loading

fykaa commented Oct 29, 2024

fykaa commented Oct 29, 2024

fykaa Oct 29, 2024

krancour Oct 29, 2024 •

edited

Loading

fykaa Oct 30, 2024

fykaa Oct 29, 2024

krancour Oct 29, 2024

fykaa Oct 30, 2024

krancour Oct 29, 2024

fykaa Oct 30, 2024

fykaa Oct 30, 2024 •

edited

Loading

krancour Oct 30, 2024

Marvin9 commented Oct 31, 2024

krancour commented Oct 31, 2024

fykaa commented Nov 4, 2024 •

edited

Loading

krancour commented Nov 4, 2024

akuitybot commented Nov 4, 2024

fix: next/prev navigation in docs #2871

fix: next/prev navigation in docs #2871

Conversation

fykaa commented Oct 29, 2024

netlify bot commented Oct 29, 2024 • edited Loading

✅ Deploy Preview for docs-kargo-io ready!

codecov bot commented Oct 29, 2024 • edited Loading

Codecov Report

krancour commented Oct 29, 2024

Marvin9 commented Oct 29, 2024 • edited Loading

krancour commented Oct 29, 2024 • edited Loading

fykaa commented Oct 29, 2024

fykaa commented Oct 29, 2024

Choose a reason for hiding this comment

krancour Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fykaa Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marvin9 commented Oct 31, 2024

krancour commented Oct 31, 2024

fykaa commented Nov 4, 2024 • edited Loading

krancour commented Nov 4, 2024

akuitybot commented Nov 4, 2024

netlify bot commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading

Marvin9 commented Oct 29, 2024 •

edited

Loading

krancour commented Oct 29, 2024 •

edited

Loading

krancour Oct 29, 2024 •

edited

Loading

fykaa Oct 30, 2024 •

edited

Loading

fykaa commented Nov 4, 2024 •

edited

Loading