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

Add page layouts functionality #439

Merged
merged 4 commits into from
Oct 21, 2018
Merged

Conversation

jamos-tay
Copy link
Contributor

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X] New feature

Fixes #389

What is the rationale for this request?

Users may want to add the same footer/head to a group of files at once without having to specify it in each file's frontMatter individually.

What changes did you make? (Give an overview)

Follow up from this solution: #389 (comment)

This PR adds support for MarkBind themes (changed from 'templates' because that name was used by nunjucks).

A theme consists of 5 files, navigation.md, head.md, footer.md, styles.css, scripts.js. Themes are stored in the _markbind/themes folder, laid out like this:

// This one is created on markbind init with all blank files - basically no change
_markbind/themes/default/
navigation.md, head.md, footer.md, styles.css, scripts.js

_markbind/themes/themeA/
navigation.md, head.md, footer.md, styles.css, scripts.js

_markbind/themes/themeB/
navigation.md, head.md, footer.md, styles.css, scripts.js

Users can apply themes to a set of pages either through the site.json or the frontMatter:

// Theme A will be applied to all index.md files
{
  "glob": "**/index.md",
  "theme": "ThemeA"
},

// Theme B will be applied to index2.md
{
  "src": "index2.md",
  "theme": "ThemeB"
},

// No Theme - default theme will be applied
{
  "src": "index3.md"
},
<frontmatter>
  theme: themeA <!-- Takes precedence over site.json -->
  head: head.md <!-- Takes precedence over themeA's head.md -->
</frontmatter>

All files in the folder will be applied.

Is there anything you'd like reviewers to focus on?

The following use cases aren't supported:

  • Using default theme for certain files only: Overriding default theme overwrites all 5 files, so you can't have say default/head.md and myTheme/footer.md at the same time.
  • Additive applying of scripts and css: Should a theme's js and css overwrite the default, or should it apply both?

@acjh
Copy link
Contributor

acjh commented Sep 24, 2018

Themes are better reserved for styling, e.g. #386.

How about "layout"?

@jamos-tay
Copy link
Contributor Author

jamos-tay commented Sep 25, 2018

Sure, layout is fine

@jamos-tay jamos-tay changed the title Add page themes functionality Add page layouts functionality Sep 25, 2018
@yamgent
Copy link
Member

yamgent commented Sep 27, 2018

Also right now the tests only includes overriding the layout in site.json. We can write additional tests to the test site for overriding the individual component of the layout in the <frontmatter> as well, e.g.:

<frontmatter>
  layout: layoutA
  head: head.md
</frontmatter>

@yamgent
Copy link
Member

yamgent commented Sep 27, 2018

  • Additive applying of scripts and css: Should a theme's js and css overwrite the default, or should it apply both?

This might result in several levels of css and js. It can go quite deep, making it hard for the user to control what should be included and what shouldn't be included.

The design can get quite complicated, but I think it is a feature certainty worth exploring in the future.

@jamos-tay
Copy link
Contributor Author

Updated with fixes (+ additional test)

Sure, we can worry about that later

@jamos-tay
Copy link
Contributor Author

Updated to resolve merge conflict

@@ -12,8 +12,9 @@
<link rel="stylesheet" href="..\markbind\css\bootstrap-glyphicons.min.css" >
<link rel="stylesheet" href="..\markbind\css\github.min.css">
<link rel="stylesheet" href="..\markbind\css\markbind.css">
<link rel="stylesheet" href="_markbind\layouts\default\styles.css">
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to put these inside the markbind folder (no underscroll at the front) rather than inside the _markbind folder, because the _markbind folder has a connotation that it contains working files, rather than files for actual deployment.

Same thing for the JavaScript files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, moved it to the assets folder.

Also added path to ignore so they don't get copied to the _markbind folder

- `scripts.js` : Contains custom javascript

These files will be automatically appended to a page upon generation, allowing you to quickly apply styles to a batch of pages at once.
Copy link
Member

Choose a reason for hiding this comment

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

Seems to be repeating the same thing as The default layout is automatically applied to every single page. below?

Or do you mean it is generated when doing a markbind init?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant to advertise the use case, like the purpose of templates is to quickly apply stuff to a batch of files.

I moved the sentence up so it's a little clearer.

@jamos-tay
Copy link
Contributor Author

jamos-tay commented Oct 10, 2018

Necessary changes made!

@yamgent yamgent added this to the v1.13.1 milestone Oct 20, 2018
@yamgent yamgent merged commit 63a07c7 into MarkBind:master Oct 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

site.json: Add configuration options to specify head, footer, site-nav for specific pages
3 participants