Upgrade docsy to 0.7.2 #569
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
size/XXL
SayakMukhopadhyay
force-pushed
the
docsy-0.7
branch
from
bf1509f to
87e7141
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
Stylistically, I have tried to keep things looking the same. Except the RSS button in the blog page. Docsy has changed its look in the PR google/docsy#1521 and this is the comparison Before: Do note, that the RSS button just had the default behaviour of Docsy and no customisation was done. If this change is not wanted, I can add the customisations necessary for changing the colour back to Orange. Or maybe we would prefer using the k8s blue. |
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
lgtm
There was a problem hiding this comment.
Wow, great work @SayakMukhopadhyay!
Inline comments are mainly suggestions to align with the new Docsy defaults. Otherwise ...
/lgtm
| .navbar-brand__name { | ||
| text-transform: uppercase; | ||
| } |
There was a problem hiding this comment.
Unless there's a feeling that this is tied to a brand-style identity (which it is not IMHO), I'd suggest that we align with the new Docsy default and drop the ALLCAPS.
|
|
||
| $display-font-sizes: ( | ||
| 1: 3rem, | ||
| 2: 2.5rem, | ||
| 3: 2rem, | ||
| 4: 1.75rem | ||
| ); | ||
|
|
||
| $display-font-weight: $font-weight-bold; |
There was a problem hiding this comment.
Again, I'd vote to align with the Docsy/Bootstrap defaults; even if this means looking slightly different from the pre-upgrade version of the site. WDYT?
|
@chalin: changing LGTM is restricted to collaborators In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chalin, SayakMukhopadhyay, sftim 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 |
|
On a related note, the instructions in https://github.com/kubernetes/contributor-site/blame/148e2934d70ecc89ab2a770407defb973dbab8d4/README.md#L66-L73 are out of date since #531. |
|
@chalin as a Docsy maintainer providing /lgtm is grounds enough for me. /hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
I will fix it in the next update. |
This PR upgrades Docsy from 0.6 to 0.7.2.
This is a a big change in Docsy as it upgraded to Bootstrap 5.2 and introduced breaking changes. See the complete change notes in https://github.com/google/docsy/releases/tag/v0.7.2
In details, the following are the changes:
autoprefixerandpostcss-cli.pl-*tops-*inlayouts/calendar/baseof.html,layouts/community/baseof.htmlandlayouts/resources/baseof.html. See https://getbootstrap.com/docs/5.2/migration/#utilities_default/content, and thelist.htmllayouts have been updated to make them the same as the ones in Docsy, apart from the needed customisations.navbar.htmlpartial has been deleted. As mentioned in the file, it was a 1:1 copy but without minification and was ok to be deleted oncetdewolff/minifywas updated to>=2.7.3. Since the dependency is not updated, the override is no longer needed._variables_project.scsshas been updated with some customisations that are needed to override some Bootstrap defaults._styles_project.scssadds any custom styles that is needed. For this release, Docsy doesn't capitalise the title in the navbar by default and capitalising it with CSS is the simplest way instead of copying the whole navbar partial.