-
Notifications
You must be signed in to change notification settings - Fork 102
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
[gen-apidocs] remove standalone navdata, use DOM instead #334
Conversation
gen-apidocs/generators/html.go
Outdated
fmt.Fprintf(html, "<LINK rel=\"stylesheet\" href=\"/css/style_apiref.css\" type=\"text/css\">\n") | ||
fmt.Fprintf(html, "<LINK rel=\"stylesheet\" href=\"css/bootstrap-4.3.1.min.css\" type=\"text/css\">\n") | ||
fmt.Fprintf(html, "<LINK rel=\"stylesheet\" href=\"css/fontawesome-4.7.0.min.css\" type=\"text/css\">\n") | ||
fmt.Fprintf(html, "<LINK rel=\"stylesheet\" href=\"css/style_apiref.css\" type=\"text/css\">\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These links are to ensure that the stylesheets can be found at k8s.io/docs website.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I had to modify them to make it easier to host the temporary files on my server. Before merging this, I will revert them :)
On that note, (how) do we need to coordinate between this repo and the website repo? Both PRs would need to be merged at roughly the same time, and then new API docs would need to be generated and copied over. What's the best way to do this cleanly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We fix the generator first, then we regenerate the reference and copy them over.
@xrstf Thanks for this cleanup. Let me know when it is ready to be merged. |
2df761e
to
1f4043c
Compare
/hold This is ready from my site, but I'm putting it on hold to coordinate the merge and deployment. |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tengqm, xrstf 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:
Approvers can indicate their approval by writing |
This is a follow-up to #333. In that PR, the navData.js generation was refactored, but I was unsure if it still functioned properly. While investigating I found that there is really no need to keep generating a standalone file, if we simply put the necessary data into the DOM from the beginning.
So that's what this PR does. It extends the generated markup a bit, by introducing
<div class="toc-item" id="...">...</div>
wrappers around each concept/section and by re-organizing the DOM for the navigation tree entirely. That way the scroll-apiref.js can deduce everything it needs from the site already.Sadly the script lives in another castle, so to show the result of this PR, I uploaded it to https://kube-api.ninja/refdocs-pr/ (using copies of the assets hosted on kubernetes.io).
kubernetes/website#43076 is the sister PR to this one, containing the rewritten
scroll-apiref.js
.