Add element tagging functionality

[jamos-tay]

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

• [X] New feature

Fixes #435

What is the rationale for this request?

The user may want to be able to show/hide certain elements from a page depending on the deploy configuration.

Cited use case:

When generating multiple sites with only slightly differing content, it might be easier to store all the information in one page, and allow the author to show/hide them. For example, if I have a Java tutorial page and a C# tutorial page, the text content can be exactly the same, only the code will differ:

# Print a line

<!-- Example syntax -->
<div tag="lang--java">
    System.out.println("Hello world");
</div>
<div tag="lang--Csharp">
    Console.WriteLine("Hello world");
</div>

The user can then deploy a Java or a C# version of the site without having two projects.

What changes did you make? (Give an overview)

Functionality implemented based on what was discussed here: #435 (comment)

Tags ({tagType}--{tagName}) are added to any HTML element.
User can specify which tags to include in site.json or frontmatter. Frontmatter takes precedence.
Only elements with tags in site.json are displayed, rest are hidden.

Simplified from include: { type: 'lang', include: ['java'] } to just include: { 'lang': ['java'] } though.

Is there anything you'd like reviewers to focus on?

Behavior works according to spec, but I'm wondering if it's a little unintuitive - do we want to mandate categorizing tags into different types (lang etc.) rather than just having general use tags, without the {type}--? I.e. include : ['tag1', 'tag2' ... ] I don't think we lose any functionality by having the latter.

[damithc]

Behavior works according to spec, but I'm wondering if it's a little unintuitive - do we want to mandate categorizing tags into different types (lang etc.) rather than just having general use tags, without the {type}--? I.e. include : ['tag1', 'tag2' ... ] I don't think we lose any functionality by having the latter.

Yes, that's fine too.

Shouldn't we allow multiple tags i.e., <div tags="java, basic, optional">

Tagged elements are shown by default or hidden by default?

[jamos-tay]

They're always hidden by default unless a matching tag is specified.

I'm not sure about multiple tags... Does that mean if a single tag matches, we display the element?

[acjh]

1. General-use tags

   without the {type}--? I.e. include : ['tag1', 'tag2' ... ]

   Yes, that's fine too.

   Agreed. tag1 should be able to contain dashes though.

2. Multiple tags

   Shouldn't we allow multiple tags i.e., <div tags="java, basic, optional">

   Yes. Shall we follow HTML class attribute syntax? i.e. <div tag="java basic optional">

   Does that mean if a single tag matches, we display the element?

   Yes, I think so.

3. Tagged elements are shown by default or hidden by default.

   They're always hidden by default unless a matching tag is specified.

   Agreed, with an important exception: if the config is undefined or null, then show all.

4. include: and "includedTags": keywords

   "renderTags": is more appropriate as it's in MarkBind's post-rendering phase.

5. Specifying in front matter

   Is there a compelling reason to specify in front matter? It seems sufficient to be in site.json.

6. Tagging functionality as a plugin

   The clean implementation makes me wonder if we can consider a plugin concept, e.g. in Eslint.
   For a start, we can have a postRender entry point. This means something like this in site.json:

   {
     "plugins": [
       "renderTags"
     ],
     "context": { // Can consider a different name
       "renderTags": ["shown", "shown-2"]
     }
   }

   And in src/plugins/render-tags.js:

   module.exports = {
     postRender: function (content, siteContext, pageConfig) {
       const renderTags = siteContext.renderTags;
       // ...
       return $.html();
     },
   };

   And in Page.js:

   - .then(result => this.filterTags(this.frontMatter.includedTags, result))
   + .then(result => this.postRender(result, siteContext, pageConfig))

   Page.prototype.postRender = function (content, siteContext, pageConfig) {
     let postRenderedContent = content;
     plugins.forEach((plugin) => { // Might be able to do `reduce` instead of `forEach`
       if (plugin.postRender) {
         postRenderedContent = plugin.postRender(postRenderedContent, siteContext, pageConfig);
       }
     });
     return postRenderedContent;
   };

7. src/ directory (can be discussed in a separate issue)

   As suggested with src/plugins/render-tags.js, let's start using src/ instead of lib/.

   • src/ is pretty common.
   • libs/ implies libraries' code, rather than this library's code. It's more nested but clearer.

   Current directory structure:

   lib/
     asset/
     markbind/
       lib/
         markdown-it/
         parser.js
         utils.js
       package-lock.json
       package.json
     template/
     util/
     Page.js
     Site.js
   package-lock.json
   package.json
   

   Proposed directory structure:

   src/
     asset/
     lib/
       markbind/
         src/
           lib/
             markdown-it/
           parser.js
           utils.js
         package-lock.json
         package.json
     template/
     util/
     Page.js
     Site.js
   package-lock.json
   package.json
   

[damithc]

2. es. Shall we follow HTML class attribute syntax? i.e. <div tag="java basic optional">

Sounds good.

2. Does that mean if a single tag matches, we display the element?

For this round, yes, we can do that. But eventually we are are going to need a exclude : ['tag1', 'tag2' ... ] and some sort of precedence (e.g., left-to-right) so as to provide more control over what gets included/excluded

4. "renderTags": is more appropriate as it's in MarkBind's post-rendering phase.

I prefer to not use include too as it is already a keyword in MarkBind. But should we use keywords that are more intuititve to the author e.g., keep/omit? render sounds like a dev term.

site.json vs frontmatter: note that tag filtering need not be site-wide. Ideally, there should be a way to specify it for a group of files.

[yamgent]

6. Tagging functionality as a plugin
The clean implementation makes me wonder if we can consider a plugin concept, e.g. in Eslint.
For a start, we can have a postRender entry point. This means something like this in site.json:
js {

I think that it is a good idea going in the direction of allowing MarkBind to be extensible in the future. Separating the core logic with the "additional" features would create a good foundation for such a plugin system.

[jamos-tay]

Changes:

Simplified tags:
tags : ['tag-1', 'tag-2']

I think we can just use the tags keyword everywhere, might be more succinct.
Removed the whole -- system, the user now just specifies a list of tags to include.
Allow multiple tags, space delimited. Element displays if at least one tag matches
All elements will be displayed if tags are not specified, or empty

I left in the front matter functionality - do we want it in? Either way it's only a couple of lines of code changed so it's not a big thing

Is having both include and exclude necessary? I think it's sufficient to either include all and the user specifies exclude, or exclude all and the user specifies include. Is there a use case where we have to have both?

Regarding the plugin thing, I think that's a great idea. It should be in its own PR though, I'll create another issue. We can migrate other functionalities (like anchors) to plugins too

[damithc]

Is having both include and exclude necessary? I think it's sufficient to either include all and the user specifies exclude, or exclude all and the user specifies include. Is there a use case where we have to have both?

While one can be used in place of the other, depending on the reuse context, one is usually more convenient than the other. Sometimes you want to say show everything except x (e.g., show everything except optional topics) in which case more tags can be added to the page and they will appear in pages without needing additional configurations in frontmatter/site.json. At other times you want to say show only x (e.g., show only java examples) in which case one can add more tags and they will not appear in the pages unless specifically requested.

[jamos-tay]

Updated

[jamos-tay]

Updated

[yamgent]

Will need you to resolve the conflicts as the lib -> src thing has been merged recently.

Some more nits in documentation.

[acjh]

Suggested improvements to phrasing and style.

[acjh]

"front matter"

"` front matter `"

[jamos-tay]

Sorry what's this one again? It's rendering like this for me:

I assume it's just removing the backticks and adding a space

[acjh]

Correct, edited.

[damithc]

Tags in the frontmatter will take precedence over the ones in site.json.

Actually, this precedence order is problematic for reuse. To paraphrase the open-closed principle from OOP, we should be able to extend existing pages (for reuse) without modifying them. i.e., we should be able to reuse a page without modifying its content while overriding its behavior using some external means, such as the site.json. What do you guys think?

[acjh]

Agreed, site.json should take precedence. I guess this followed Layouts, which also needs this change.

[jamos-tay]

Sure - can change the order. Will change the other one in a separate PR.

Updated

[jamos-tay]

Updated

[yamgent]

Comments about the documentation.

[jamos-tay]

Updated

[yamgent]

@jamos-tay can you resolve conflict for this PR? Thanks!

[jamos-tay]

Rebased on master!

[damithc]

@jamos-tay is it possible to override frontmatter tags of a specific page/glob from site.json? Or site.json tags can only apply to the entire site?

[yamgent]

@jamos-tay is it possible to override frontmatter tags of a specific page/glob from site.json? Or site.json tags can only apply to the entire site?

Currently it applies to the entire site only, no way to do it for a particular page.

The current overriding mechanism is also rather aggressive atm: I realize that it is not true that we don't lose any functionality for not categorizing tags into different groups. Consider this scenario:

---

helloWorld-fragment.mbdf:

<div tags="level--beginner">

This is how you do Hello World:

<code tags="lang-cs">Console.WriteLine("Hello World");</code>
<code tags="lang-java">System.out.println("Hello World");</code>

</div>

<div tags="level--advanced">

Let's do a refresher. Recall that to print anything, you would use <code tags="lang-cs">Console.WriteLine()</code><code tags="lang-cs">println()</code>.

<question>
How would you print the string "Hello World"?
  ...
</question>

</div>

index-cs.mbd:

<frontmatter>
    tags: lang-cs
</frontmatter>

<include src="helloWorld-fragment.mbdf" />

index-java.mbd:

<frontmatter>
    tags: lang-java
</frontmatter>

<include src="helloWorld-fragment.mbdf" />

I want two different websites: for beginner, and for advanced. Both of these websites will render index-cs.mbd and index-java.mbd. I have no way of specifying either level--beginner or level--advanced without accidentally overriding the lang--* tags.

[damithc]

@jamos-tay perhaps we can do another iteration for this feature to iron out these rough edges?

[jamos-tay]

Hmm... I think what we can do is a tag merge, i.e. instead of overwriting the frontmatter tags completely we merge them instead. We probably should have done it this way earlier.

[damithc]

@jamos-tay is it possible to override frontmatter tags of a specific page/glob from site.json? Or site.json tags can only apply to the entire site?

@jamos-tay don't forget this one too.

In addition, should we also have a way to specify tags at the point of inclusion, the same way we allow overriding variables at the point of inclusion?