-
Notifications
You must be signed in to change notification settings - Fork 123
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
Allow using 'none' footer attribute in frontmatter #1002
Conversation
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.
Overall approach seems okay.
Even though it might not be in the scope of this PR, wondering whether this functionality should only be for footer? @damithc
src/Page.js
Outdated
@@ -594,6 +594,12 @@ Page.prototype.insertHeaderFile = function (pageData) { | |||
Page.prototype.insertFooterFile = function (pageData) { | |||
const { footer } = this.frontMatter; | |||
let footerFile; | |||
|
|||
if (footer === 'none') { | |||
// avoid adding any footer |
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.
comment seems redundant
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.
will remove it 👍
src/Page.js
Outdated
@@ -594,6 +594,12 @@ Page.prototype.insertHeaderFile = function (pageData) { | |||
Page.prototype.insertFooterFile = function (pageData) { | |||
const { footer } = this.frontMatter; | |||
let footerFile; | |||
|
|||
if (footer === 'none') { |
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.
if 'none' is a keyword, put it as a constant.
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.
will make the change 👍
There was a suggestion to extend this to other elements. I have no objections to that. I'll leave the decision to senior devs. |
008996e
to
27c1a26
Compare
Ready for review |
* 'master' of https://github.com/MarkBind/markbind: Update tests Allow using 'none' footer attribute in frontmatter (MarkBind#1002) Support line numbers for code blocks (MarkBind#991) 2.11.0 Update test files due to changes in PR MarkBind#982 Update vue-strap version to v2.0.1-markbind.36 Make highlighting bold (MarkBind#1045) Support markdown for header attr in dropdown (MarkBind#1029) Add '_site' to the ignored folders in site.json (MarkBind#1046) Use path.join instead of string interpolation (MarkBind#1052) Implement box markdown header attributes parsing (MarkBind#1025) Make the position of top navbar fixed (MarkBind#982) Exclude *.md files from being copied over on build (MarkBind#1010) # Conflicts: # docs/css/main.css
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
Fixes #638
What is the rationale for this request?
Enables users to exclude the footer that is derived/inherited from the layout specified in the
<frontmatter>
.What changes did you make? (Give an overview)
Added a check for when the
footer
attribute isnone
while inserting the footer file into aPage
.Testing instructions:
<frontmatter>
footer: none
to<frontmatter>
Expected output: Footer of the derived layout is omitted
Proposed commit message: (wrap lines at 72 characters)
Allow using 'none' footer attribute in frontmatter