Skip to content
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

Upgrade mkdocs and mkdocs-material #122

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

trickeydan
Copy link
Contributor

Upgrades mkdocs and other dependencies, notably upgrading mkdocs-material to v7.

Also updated mkdocs.yml for slight changes in config. Some icons are slightly different due to the old icon-set no longer being available.

Help Requested: I am unable to work out why the menu structure doesn't render as intended.

Preview when rendered: https://static.trickey.io/runbook/

(I ended up on this adventure when attempting to do #2 as a quick thing)

@trickeydan trickeydan added help wanted Extra attention is needed dependencies Pull requests that update a dependency file labels Oct 3, 2021
@RealOrangeOne
Copy link
Member

I am unable to work out why the menu structure doesn't render as intended.

I remember diving into this a while back. I don't believe it is possible anymore. It appears it broke around the time the navigation was rewritten in mkdocs-material (6.2.0).

After digging around this again, I've submitted an issue upstream: squidfunk/mkdocs-material#3076

language: 'en'
features:
- navigation.tabs
- navigation.instant
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: We should add the navigation.index feature. It means the dropdowns are clickable and have navigation, which will be incredibly helpful for some of the content we write.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, though perhaps not as part of this upgrade PR?

@PeterJCLaw
Copy link
Member

Aside: I don't think your repro needs navigation.instant

Copy link
Member

@PeterJCLaw PeterJCLaw left a comment

Choose a reason for hiding this comment

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

(I was digging into the nav issues too, but @RealOrangeOne beat me to an answer)

Comment on lines +7 to +11
name: 'material'
icon:
logo: fontawesome/solid/book
favicon: 'favicon.ico'
language: 'en'
Copy link
Member

Choose a reason for hiding this comment

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

nit: does the ordering & quoting of these matter? (Wondering whether the diff can be reduced by keeping the previous ordering & removing the quotes.)

language: 'en'
features:
- navigation.tabs
- navigation.instant
Copy link
Member

Choose a reason for hiding this comment

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

nit: Is this a required part of the upgrade?

language: 'en'
features:
- navigation.tabs
- navigation.instant
Copy link
Member

Choose a reason for hiding this comment

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

Agree, though perhaps not as part of this upgrade PR?

@PeterJCLaw PeterJCLaw mentioned this pull request Oct 3, 2021
@RealOrangeOne
Copy link
Member

From the related issue, it's no longer possible to have pages like we currently have them.

Personally I'd suggest we have a small meta section about the runbook itself which houses these page, or we attempt to bake them into he homepage page itself (that'll get quite long).

@PeterJCLaw
Copy link
Member

A breaking change removing something that is actually kinda hard to achieve another way occurring in a non-major release and then relegated to a footnote is rather disappointing IMO.

As I understand it the alternatives are:

  • move the content to a meta folder and accept that it won't show up on the root page (plus maybe find some way to hide that folder from the tabs)
  • some kind of redirect so that there is no actual root page and instead the root is /meta (would need to separately work out how to ensure that the meta directory's entry is the first tab in the list)
  • fork the theme and/or override some aspect of it
  • a plugin to adjust the nav (is that possible?)
  • fully specify the entire nav tree in the config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants