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

Added 2 templates #1

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Added 2 templates #1

wants to merge 12 commits into from

Conversation

eecollante10
Copy link

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
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
Author

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

@brrd
Copy link
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.

None yet

2 participants