search

trustoverip / mkdocs-material

ToIP repo template for mkdocs with ToIP Material Theme to be used as a GitHub Template Repository.
Apache License 2.0
1 stars 1 forks source link

First Draft Review #1

Closed vinomaster closed 3 years ago

vinomaster commented 3 years ago

I have created a fork of this repo and have run some tests to get acquainted with the approach.

Styling

It seems very vanilla... with small logo. The Linux Foundation gets for visibility than ToIP. Can we improve the theme? Minimally can we get to this level of theme/logo using this example Logo URL?

Additionally the theme for specs needs to look like this example but using our color palette. For example: Table of Contents header background should use one of our blues and the grey left-nav background should use something from our palette.

Separation of Theme from Content

Can we move /doc/assets to /theme/toip thereby allowing for flexibility in YAML?

Decoupling Make and YAML Files

We want to ensure flexibly in configuration. For example what about these YAML file options and how do we make sure the Makefile is not impacted? The make should not assume the site_dir or docs_dir.

docs_dir: 'content'
site_dir: 'html'
dev_addr: '127.0.0.1:8800'

GitHub Template Repo

  1. Update repo to be a Template Repo. In other words, creation of a new repo should be pushing a GitHub button not following instructions... we need to keep this simple.
  2. Modify README.md instructions accordingly.
  3. See example button and instructions here.

Make Help

Add make help like this example which parses the Makefile header.

Setup Step 4

Preparing the local environment needs to be simplified via a make command. See example here. It is ok if you want to assume MacOS as target env.

Make Setup

Do not get the (venv) prompt -- maybe b/c my bash_profile overirides?

Successfully built pandocfilters
Installing collected packages: mkdocs-enumerate-headings-plugin, pandocfilters
Successfully installed mkdocs-enumerate-headings-plugin-0.4.3 pandocfilters-1.4.3

[~/dev/GitHub/ToIP/forks/mkdocs-material-eval]
==> ls -l
total 72
-rw-r--r--   1 dag  staff  11357 Nov 25 10:47 LICENSE
-rw-r--r--   1 dag  staff   1689 Nov 25 10:47 Makefile
-rw-r--r--   1 dag  staff   3611 Nov 25 10:47 README.md
-rw-r--r--   1 dag  staff   2519 Nov 25 10:47 REQUIREMENTS.md
drwxr-xr-x  11 dag  staff    352 Nov 25 11:23 docs
-rw-r--r--   1 dag  staff   3223 Nov 25 10:47 mkdocs.spec.yml
-rw-r--r--   1 dag  staff   3123 Nov 25 10:47 mkdocs.yml
drwxr-xr-x   3 dag  staff     96 Nov 25 10:47 overrides
-rw-r--r--   1 dag  staff    607 Nov 25 10:47 requirements.txt
drwxr-xr-x   4 dag  staff    128 Nov 25 10:47 scripts
drwxr-xr-x   6 dag  staff    192 Nov 25 11:24 venv

[ ~/dev/GitHub/ToIP/forks/mkdocs-material-eval]
==>

Make Combine

  1. Trivial -- fix "your" spelling in readme.
  2. Did not work ... result was empty index.md
  3. The approach of using find docs -name "*.md" does not allow for the authors to convey ordering of files. Why not simply leverage the nav block in the yaml?

PDF Generation

Where are plugins for this? The make should be extended to generate a PDF in the dist folder via similar pandoc methods or other mkdocs plugins.

squidfunk commented 3 years ago

Thanks for the feedback, here are my thoughts:

Styling It seems very vanilla... with small logo. The Linux Foundation gets for visibility than ToIP. Can we improve the theme? Minimally can we get to this level of theme/logo?

We can adjust the styling, for sure. I analyzed https://trustoverip.org/ and tried to provide a styling which is as close as possible to your CI, while being easy on the eyes. I settled with your brand colors for links, fonts (it was Roboto after all, which is the default of Material for MkDocs) and a light appearance in general. We can increase the height of the header, but it will not collapse without any additional effort when scrolling down, so it will remain at this size.

Can we move /doc/assets to /theme/toip thereby allowing for flexibility in YAML?

What do you mean by flexibility in YAML? In the end, the assets (JS, CSS, logo) need to be under docs_dir, because that's where MkDocs expects them to be when building the site.

We want to ensure flexibly in configuration. For example what about these YAML file options and how do we make sure the Makefile is not impacted? The make should not assume the site_dir or docs_dir.

Passing parameters to make is only possible via environment variables - it's not possible to pass arguments otherwise. Thus, we'd need to add env variables with reasonable defaults. However, that would either mean we need to switch to a two-pass Makefile, i.e. second expansion, as targets would be dynamic, or move to phony targets, only, thus losing the upside of using a Makefile, that is dependency tracking. Another idea would be to parse the data directly from the mkdocs.yml, by using yq or something similar. I haven't tried yqyet (as I wanted to keep dependencies as small as possible), but unfortunately, some YAML parsers have problems parsing mkdocs.yml because of the dynamic !!python values.

Update repo to be a Template Repo. In other words, creation of a new repo should be pushing a GitHub button not following instructions... we need to keep this simple.

Unfortunately, as a maintainer (and not an admin) I don't have the rights to do this. I cannot change this repository to a template repository, that's something that has to be done by an admin. The instructions are meant to be for when you check out the repository and run it locally.

Add make help like this example which parses the Makefile header.

Check, we can do that.

Preparing the local environment needs to be simplified via a make command. See example here. It is ok if you want to assume MacOS as target env

It wasn't clear to me that we can assume macOS. I'll add the command from your repository.

Do not get the (venv) prompt -- maybe b/c my bash_profile overirides?

That could be a shell/local difference. Can you execute the commands successfully? In that case it will work nonetheless.

Trivial -- fix "your" spelling in readme.

Will do.

Did not work ... result was empty index.md

Did you add the files you created to the nav entry in mkdocs.yml? An excerpt from the README:

If you want to create a combined Markdown file, this repository contains some scripts which will preprocess and concatenate all Markdown files in the order specified in mkdocs.yml. The scripts will ensure that internal links to pages and anchors are correct. The combined file can be created with:

The approach of using find docs -name "*.md" does not allow for the authors to convey ordering of files. Why not simply leverage the nav block in the yaml?

The nav block is leveraged, as noted in the comment above. The find docs... command is only used to detect changes, so you the target needs to be rebuilt. The actual logic is here:

https://github.com/trustoverip/mkdocs-material/blob/5c6bafda0b1906cc486fa51968964def743e0456/scripts/combine.sh#L17-L38

Where are plugins for this? The make should be extended to generate a PDF in the dist folder via similar pandoc methods or other mkdocs plugins.

As I noted via Slack, I have some questions regarding the PDF, so I did not include it yet.

vinomaster commented 3 years ago

On the topic of Separation of Theme from Content could we MOVE all Theme content to docs_dir where default is /docs

/docs
   /theme
       /assets
       /css
       /overrides

Then update the YAML accordingly. This way of docs_dir is set to something other than default and /docs folder is renamed....no impact.

vinomaster commented 3 years ago

Passing parameters to make is only possible via environment variables.

Lets discuss. I beleive thsi is possible via command line overrides and properly setup Makefiles using variable exporting.

For example:


Makefile:
      DOCSDIR?='./docs'

      dist/assets: $(DOCSDIR)/assets $(shell find $(DOCSDIR)/assets)

Command Line:
      make combine DOCSDIR='/content'
vinomaster commented 3 years ago

Template Repo Button:

I will handle this change once we close on rest of work. Final Step :-)

vinomaster commented 3 years ago

When we syncup, we can review the issues with virtualenv .... regardless this approach places a burden on repo user to know how to install dependences on MacOS or Windows.

The goal is to allow an author that is not a developer to simply have an environment that can be used locally whereby they edit markdown files and can instantly see changes to locally running site in a browser.

If we can create a Makefile that hides Docker commands this should be possible using:

Platform agnostic Preparation

  1. Install Docker.

Make Commands

make serve which will

  1. Create a volume mount binding the locally cloned repo folder
  2. Pull a ToIP Modified Docker Image
  3. Run mkdocs serve

This will:

squidfunk commented 3 years ago

Zoom Call Notes 27-NOV:

  1. Logo bigger, use logo in collapsed state as on ToIP landing page(fixed in fdb5b52)
  2. Make header title configurable (extra.title, otherwise use site_name) (fixed in c24d046)
  3. Check if we can move overrides to docs/theme/overrides(fixed in 105ce6a)

Regarding the spec theme:

  1. Remove table of contents, stretch to whole width
  2. Adjust spec theme to look more like SpecUp

Additionally:

  1. Combine script must respect quotation marks in nav structure (fixed in ea49582)
  2. Render combined Markdown as PDF with Pandoc (as Makefile target)
  3. Add help rule to Makefile (resolved by @vinomaster)
squidfunk commented 3 years ago

As discussed, the specification styles now mirror the SpecUp information architecture:

screenshot-squidfunk-github-io-toip-demo-spec-1606727254098

Click on the screenshot to see the demo

vinomaster commented 3 years ago

Spec demo looks good...

two comments: a) Is it possible for the Linux Foundation header to be left justified above left-nav frame?

b) We discussed the expansion of per/page sections in left-nav

1. Title
2. blal
   2.1
   2.2
3. blah
4. blah

where when Section 2 is in focus the subsections are expanded and then removed when not in focus. So only the current page is expanded. This was to make up for elimination of right table of contents per page nav.

squidfunk commented 3 years ago

a) Fixed by 65a53cb b) Fixed by d894edf

Both deployed on https://squidfunk.github.io/toip-demo-spec/

vinomaster commented 3 years ago

Looks Great!