Skip to content
This repository was archived by the owner on Nov 21, 2018. It is now read-only.

Added 2 templates#1

Open
eecollante10 wants to merge 12 commits into
brrd:masterfrom
eecollante10:patch-1
Open

Added 2 templates#1
eecollante10 wants to merge 12 commits into
brrd:masterfrom
eecollante10:patch-1

Conversation

@eecollante10
Copy link
Copy Markdown

I added two templates which use strapdown js.
The strapdownMenu one adds a menu on the top left with links to places on the document.

Added html and json template files for strapdownMenu template
Added css and js files to assets of strapdownMenu template
Added html and son template files for strap down template
@brrd
Copy link
Copy Markdown
Owner

brrd commented Jun 22, 2016

Hi,

I'm sorry, I really don't understand why you are using strapdown.js here?

$DOCUMENT_CONTENT already contains HTML, so you don't need to reprocess it again using strapdown or anything else. This is working because HTML is supported in markdown, but this creates useless dependencies and leads to bad performances and weird markup.

Some other remarks:

  • Please use English names for your variables, id, class, comment, etc. in order to keep this project accessible to anyone.
  • Please don't repeat the document title twice in the header.
  • For some reason (probably because charset tag is missing) character encoding is messed up.
  • Please use clear and explicit names in menu labels.
  • Please don't use confusing keybindings (such as Ctrl+P which is reserved for Print in most applications).
  • Please don't comment useless copy-pasted markup (like this), remove it instead.
  • Please don't console.log things in your scripts.

I'm sorry but I won't merge this unless you do a substantial clean up of this code.

@eecollante10
Copy link
Copy Markdown
Author

What do you mean by: Please use clear and explicit names in menu labels.

@brrd
Copy link
Copy Markdown
Owner

brrd commented Jun 22, 2016

What do you mean by: Please use clear and explicit names in menu labels.

Personally I don't find "Md With Menu template" name very clear.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants