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

[RFE] Add class on ul/ol wrapper of task-list? #113

Closed
4 tasks done
cipherboy opened this issue Mar 9, 2020 · 3 comments
Closed
4 tasks done

[RFE] Add class on ul/ol wrapper of task-list? #113

cipherboy opened this issue Mar 9, 2020 · 3 comments

Comments

@cipherboy
Copy link
Contributor

I'd like to consider adding classes to containers of extensions. Consider a task list:

- [ ] item 1
- [ ] item 2

This'll get rendered into the following HTML (for instance -- I didn't execute goldmark here):

<ul>
  <li><input type="checkbox" checked=""> item 1</li>
  <li><input type="checkbox" checked=""> item 1</li>
</ul>

There's no way to differentiate (in CSS) between a <ul> with a task list underneath and a regular bulleted list. pandoc solves this problem by adding a class="task-list" to the <ul>. GitHub adds the class contains-task-list.

I think we could solve this in three ways:

  1. Directly extend the <ul> rendering classes to detect if it contains a task-list. Solve it for this one case.
  2. Add a more generic mechanism for extensions to inform the AST of modifications to parent elements. Converting e.g., <p> to <div>, adding arbitrary number of classes on arbitrary parent elements, ... -- gives control of modifying the AST to extensions rather than external callers.
  3. Add a configuration parameter that allows the calling application to configure what classes they'd like to have on what classes. Allow introspection of the children elements so the application can modify it as they desire. Probably involves walking the AST and executing a callback function on each step.

Thoughts? I'm happy to implement whatever you decide, I'd just like to leave the design decision to you as it is your project.


Boilerplate below:

  • goldmark is fully compliant with the CommonMark. Before submitting issue, you must read CommonMark spec and confirm your output is different from CommonMark online demo. task-lists are not part of CommonMark and are a part of the gfm spec.
    • Extensions(Autolink without < >, Table, etc) are not part of CommonMark spec. You should confirm your output is different from other official renderers correspond with an extension. pandoc renders an unordered task list with <ul class="task-list">. GitHub uses the class contains-task-list.
  • goldmark is not dedicated for Hugo. If you are Hugo user and your issue was raised by your experience in Hugo, you should consider create issue at Hugo repository at first .
  • Before you make a feature request, you should consider implement the new feature as an extension by yourself . To keep goldmark itself simple, most new features should be implemented as an extension. The core of this feature already exists as an extension, I'd like to discuss changing how it occurs though. Happy to help implement it once I understand goldmark better and once the design for the feature becomes clear.

Please answer the following before submitting your issue:

  1. What version of goldmark are you using? : v1.1.25.
  2. What version of Go are you using? : v1.13
  3. What operating system and processor architecture are you using? : linux/amd64
  4. What did you do? : Rendered a task list (like the above).
  5. What did you expect to see? : I hoped for a class on the parent ul/ol element, to different it from a regular bulleted/numbered list.
  6. What did you see instead? : a bare ul/ol.
  7. Did you confirm your output is different from CommonMark online demo or other official renderer correspond with an extension?: Yes, GitHub and pandoc give the ul a class, to help with styling.
  8. (Feature request only): Why you can not implement it as an extension?: I'd like to. I need to understand what the scope of this feature should be, before implementing it. I'm filing this issue to begin a discussion.
    • You should avoid saying like "I'm not familiar with this project" "I'm not a Go programmer" as far as possible. This is an open source project and a library for Go programmers. I encourage you to strive to read source codes.
    • I absolutely welcome questions that are difficult even if you read the source codes.
@yuin
Copy link
Owner

yuin commented Mar 10, 2020

pandoc renders an unordered task list with <ul class="task-list">. GitHub uses the class contains-task-list.

Official specification is here and as you can see task lists do not have any classes. It means pandoc and Github implementation do not stick with official spec. So this should be an extension and not be included in the main repository.

I would recommend you to implement an ASTTransformer.

  1. Walk through AST using ast.Walk
  2. If we find an ast.TaskCheckBox node,
    1. Get an ast.List node by n.Parent().Parent()
    2. Add classes: listNode.SetAttributeString("class", "task-list")

@zeripath
Copy link

zeripath commented Mar 22, 2020

Thanks @yuin I thought this might be your suggested solution. I've implemented something similar in our transformer. I guess that this issue could now be closed.

Ref: go-gitea/gitea#10798

@yuin
Copy link
Owner

yuin commented Mar 23, 2020

@zeripath That's good to know, thanks!

@yuin yuin closed this as completed Mar 23, 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

No branches or pull requests

3 participants