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

Fix reload inconsistency when updating frontmatter #1068

Merged
merged 2 commits into from
Mar 7, 2020
Merged

Fix reload inconsistency when updating frontmatter #1068

merged 2 commits into from
Mar 7, 2020

Conversation

Parcly-Taxel
Copy link
Contributor

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

What is the rationale for this request?
See proposed commit message below. This fixes #1001.

What changes did you make? (Give an overview)
The frontmatter files for the site templates are populated.

Is there anything you'd like reviewers to focus on?
This is just #1035 reopened, with proper branch management on my part, following advice from @ang-zeyu.

Testing instructions:
See topic opener for #1001.

Proposed commit message: (wrap lines at 72 characters)
Fix reload inconsistency when updating frontmatter

The header, footer and navigation frontmatter for the minimal and
default site templates are empty. This causes inconsistent reloading
when the frontmatter is changed (#1001). Here we populate said
frontmatter.

The header, footer and navigation frontmatter for the minimal and
default site templates are empty. This causes inconsistent reloading
when the frontmatter is changed (#1001). Here we populate said
frontmatter.
@crphang
Copy link
Contributor

crphang commented Feb 29, 2020

@Parcly-Taxel could we also add this to the collectAllPageSections?

Page.prototype.collectAllPageSections = function () {
+ this.pageSectionsHtml = {}; // This resets the pageSectionsHTML whenever we collect.
  this.collectPageSection('header');
  this.collectPageSection(`#${SITE_NAV_ID}`);
  this.collectPageSection('footer');
};

This ensures that each reload always reset the pageSections and reduce the dependency on how templates are created which could potentially be crowd-sourced. While you're at it, could you also add <header> and <footer> to minimal template?

Thanks for your contribution!

@Parcly-Taxel
Copy link
Contributor Author

could we also add this to the collectAllPageSections?

Page.prototype.collectAllPageSections = function () {
+ this.pageSectionsHtml = {}; // This resets the pageSectionsHTML whenever we collect.
  this.collectPageSection('header');
  this.collectPageSection(`#${SITE_NAV_ID}`);
  this.collectPageSection('footer');
};

@crphang done, but @ang-zeyu said in #1035 to "go the other way", i.e. "add <header> and <footer> to minimal template", which I had already done before closing the original PR.

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Mar 1, 2020

@crphang done, but @ang-zeyu said in #1035 to "go the other way", i.e. "add <header> and <footer> to minimal template", which I had already done before closing the original PR.

You could do both #1001 (comment) :)

The fix I suggested was merely to remove {{ timestamp }} from the footer(s), which would cause your tests to fail irregardless
And also to add the changes to the minimal template for consistency since the default template is also getting "the other fix". ( markbind has 2 templates )

@Parcly-Taxel
Copy link
Contributor Author

Parcly-Taxel commented Mar 1, 2020

You could do both #1001 (comment) :)

The fix I suggested was merely to remove {{ timestamp }} from the footer(s), which would cause your tests to fail irregardless
And also to add the changes to the minimal template for consistency since the default template is also getting "the other fix". ( markbind has 2 templates )

@ang-zeyu yes, can we merge this now, since I have done both?

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Mar 1, 2020

yes, can we merge this now, since I have done both?

Looks good to me 👍

I don't have merging powers here though, you'd have to wait for a maintainer to merge this in!

@Parcly-Taxel
Copy link
Contributor Author

yes, can we merge this now, since I have done both?

Looks good to me +1

I don't have merging powers here though, you'd have to wait for a maintainer to merge this in!

Can't you approve it first?

@yamgent yamgent added this to the v2.11.1 milestone Mar 3, 2020
@yamgent yamgent merged commit 0bf33ae into MarkBind:master Mar 7, 2020
Tejas2805 added a commit to Tejas2805/markbind that referenced this pull request Mar 7, 2020
…nvert-to-code-block

* 'master' of https://github.com/MarkBind/markbind:
  Allow changing parameter properties (MarkBind#1075)
  Custom timezone for built-in timestamp (MarkBind#1073)
  Fix reload inconsistency when updating frontmatter (MarkBind#1068)
  Implement an api to ignore content in certain tags (MarkBind#1047)
  Enable AppVeyor CI (MarkBind#1040)
  Add heading and line highlighting to code blocks (MarkBind#1034)
  Add dividers and fix bug in siteNav (MarkBind#1063)
  Fixed navbar no longer covers modals (MarkBind#1070)
  Add copy code-block plugin (MarkBind#1043)
  Render plugins on dynamic resources (MarkBind#1051)
  Documentation for Implement no-* attributes for <box>  (MarkBind#1042)
  Migrate to bootstrap-vue popovers (MarkBind#1033)
  Refactor preprocess and url processing functions (MarkBind#1026)
  Add pageNav to Using Plugins Page (MarkBind#1062)

# Conflicts:
#	docs/userGuide/syntax/siteNavigationMenus.mbdf
Tejas2805 added a commit to Tejas2805/markbind that referenced this pull request Mar 9, 2020
* 'master' of https://github.com/MarkBind/markbind:
  2.12.0
  Update outdated test files
  Update vue-strap version to v2.0.1-markbind.37
  Fix refactor to processDynamicResources (MarkBind#1092)
  Implement lazy page building for markbind serve (MarkBind#1038)
  Add warnings for conflicting/deprecated component attribs (MarkBind#1057)
  Allow changing parameter properties (MarkBind#1075)
  Custom timezone for built-in timestamp (MarkBind#1073)
  Fix reload inconsistency when updating frontmatter (MarkBind#1068)
  Implement an api to ignore content in certain tags (MarkBind#1047)
  Enable AppVeyor CI (MarkBind#1040)
  Add heading and line highlighting to code blocks (MarkBind#1034)
  Add dividers and fix bug in siteNav (MarkBind#1063)
  Fixed navbar no longer covers modals (MarkBind#1070)
  Add copy code-block plugin (MarkBind#1043)
  Render plugins on dynamic resources (MarkBind#1051)
  Documentation for Implement no-* attributes for <box>  (MarkBind#1042)
  Migrate to bootstrap-vue popovers (MarkBind#1033)
  Refactor preprocess and url processing functions (MarkBind#1026)
  Add pageNav to Using Plugins Page (MarkBind#1062)
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.

Inconsistent reloading of site on updating frontmatter
4 participants