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

Simplify site nav generation #1221

Merged
merged 1 commit into from
Jun 7, 2020
Merged

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented May 4, 2020

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

• [ ] Documentation update
• [ ] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [x] Other, please explain:

Code maintainability

What is the rationale for this request?

  • simplify the recursive algorithm used in site nav generation to an iterative one
  • should also bring some performance benefits since we are no longer constantly reparsing / outputting the html across recursive calls of formatSiteNav

What changes did you make? (Give an overview)

  • merged formatSiteNav into insertSiteNav since the recursion is removed
  • the new algorithm simply iterates through ul elements one by one, and its direct li child elements, formatting it the same way the old algorithm did
  • extract some more constants to constants.js

Testing instructions:

  • npm run test should pass ( no test file updates here )

Proposed commit message: (wrap lines at 72 characters)
Simplify site nav generation

The site nav generation algorithm is heavily recursive.

Let's remove the recursion in favour of a simple iterative process.

const siteNavContent = fs.readFileSync(siteNavPath, 'utf8');
if (siteNavContent === '') {
Copy link
Contributor Author

@ang-zeyu ang-zeyu May 4, 2020

Choose a reason for hiding this comment

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

handled by navigationElements.length === 0 below

@ang-zeyu ang-zeyu force-pushed the site-nav-refactor branch 3 times, most recently from 9a0a7bf to b1ecb97 Compare May 5, 2020 08:15
@ang-zeyu ang-zeyu requested a review from yamgent May 19, 2020 15:48
The site nav generation algorithm is heavily recursive.

Let's remove the recursion in favour of a simple iterative process.
@ang-zeyu ang-zeyu added this to the v2.15.0 milestone Jun 7, 2020
@ang-zeyu ang-zeyu merged commit 4140a70 into MarkBind:master Jun 7, 2020
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.

1 participant