hexojs / hexo

A fast, simple & powerful blog framework, powered by Node.js.
https://hexo.io
MIT License
39.49k stars 4.85k forks source link

Only one class is allowed on `list_tags` helper #3226

Closed Xaviju closed 6 years ago

Xaviju commented 6 years ago

Feature request

Allow multiple classes on list_tags helper

Currently only one class is allowed on thelist_tags helper because it transforms the class and applies classes to this element and its children.

Would be quite easy and useful to allow multiple classes in an array and apply all transformations in the HTML generation, but would like to listen to your opinion before digging into code.

tcrowe commented 6 years ago

Hey @Xaviju :wave:

What happens if you used class1 class2 class3 on the list_tags class option?

Xaviju commented 6 years ago

Hi @tcrowe

:thinking: It currently works, but not as expected. Let me explain:

According to this code it will get the string on the list_tags class option and applies a string to it.

Example

If I add multiple classes like this:

<%- list_tags({
    class: "main-tag secondary-tag"
}) %>

Current generated code

<ul class="main-tag secondary-tag-list">
    <li class="main-tag secondary-tag-list-item">
        <a class="main-tag secondary-tag-list-link" href="#">Something</a>
    </li>
</ul>

Expected

<ul class="main-tag secondary-tag tag-list">
    <li class="main-tag secondary-tag tag-list-item">
        <a class="main-tag secondary-tag tag-list-link" href="#">Something</a>
    </li>
</ul>

Sorry if the issue is not well explained, please feel free to edit it

tcrowe commented 6 years ago

I tested it and I see that it always appends -list to the class.

https://github.com/hexojs/hexo/blob/92baaada795f6bdfb76ccb6c36181ad9b64a89c4/lib/plugins/helper/list_tags.js#L35

For now want to try an alternative?

./source/_posts/hello-world.md

---
title: Hello World
categories:
- catone
- cattwo
- catthree
tags:
- tagone
- tagtwo
- tagthree
---
Welcome to [Hexo](https://hexo.io/)!

./themes/your-theme/layout/_partial/post-tag-list.ejs


<!--

for post tags only. page tags are different

-->

<div class="<%=containerClass || 'post-tag-list'%>">
  <% page.tags.each(tag => { %>
    <div class="<%=itemClass || 'post-tag-item'%>">
      <a class="<%=linkClass || 'post-tag-link'%>" href="<%=url_for(tag.path)%>">
        <%=tag.name%>
        <span class="<%=countClass || 'post-tag-count'%>"><%=tag.length%></span>
      </a>
    </div>
  <% }) %>
</div>

./themes/your-theme/layout/post.ejs

<%- partial('_partial/article', {post: page, index: false}) %>

<%-partial('_partial/post-tag-list', {
  containerClass: 'custom-tag-list',
  itemClass: 'custom-tag-item',
  linkClass: 'custom-tag-link',
  countClass: 'custom-tag-count'
})%>
Xaviju commented 6 years ago

@tcrowe thanks for the alternative method.

I tested it and I see that it always appends -list to the class. Not only to the parent element, also to the child elements in the list tree.

Should the helper be fixed? I can try to do it :)

tcrowe commented 6 years ago

I guess I'm not sure what the original intent was behind that helper class option. 😬

Maybe someone who knows can tell us. git blame ... might help us find who to ask.

Xaviju commented 6 years ago

According to the file history, the list_tags helper class list was created on this commit and has never been modified since then (https://github.com/hexojs/hexo/commit/bcdfb298196c3b874383ff3790d99317ac895766#diff-8261f6bff72a6e1eb8a1c5307939a28d) by @tommy351 maybe he might help us.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because lack of recent activity. It will be closed if no further activity occurs. Thank you for your contributions.