Add TODO list support for Blackfriday #2296

Merged
merged 6 commits into from Sep 9, 2016

Projects

None yet

5 participants

@bep
Collaborator
bep commented Jul 21, 2016 edited

Fixes #2269

  • Basic support
  • A flag to turn it off
  • Add a way to style the list bullet away
  • A test
  • Some documentation
@bep bep changed the title from Add TODO list support for Blackfriday to WIP: Add TODO list support for Blackfriday Jul 21, 2016
@bep
Collaborator
bep commented Jul 21, 2016

screen shot 2016-07-21 at 18 46 27

But we need to add a CSS-class somewhere so it is possible to hide the list bullet (as GitHub does).

/cc @phtan

@bep
Collaborator
bep commented Jul 21, 2016

This is how GitHub does it for the li-item:

class="task-list-item enabled"
@bep bep added a commit to bep/hugo that referenced this pull request Jul 21, 2016
@bep bep Add CSS class to TODO list and list items
See #2296
0afd044
@bep bep added a commit to bep/hugo that referenced this pull request Jul 21, 2016
@bep bep Add CSS class to TODO list and list items
See #2296
d5196a3
@bep bep added a commit to bep/hugo that referenced this pull request Jul 21, 2016
@bep bep Add CSS class to TODO list and list items
See #2296
e41ef73
@bep bep added a commit to bep/hugo that referenced this pull request Jul 21, 2016
@bep bep Add CSS class to TODO list and list items
See #2296
dd2abff
@bep bep added a commit to bep/hugo that referenced this pull request Jul 22, 2016
@bep bep Add CSS class to TODO list and list items
See #2296
5a20078
@bep bep added a commit to bep/hugo that referenced this pull request Jul 22, 2016
@bep bep Add a flag to turn task list support off
See #2296
a7ccd6a
@bep bep added a commit to bep/hugo that referenced this pull request Jul 22, 2016
@bep bep Add tests for Blackfriday task lists
See #2296
2c6809f
@bep bep added a commit to bep/hugo that referenced this pull request Jul 22, 2016
@bep bep Document Blackfriday task lists
See #2296
2c541a8
@bep bep changed the title from WIP: Add TODO list support for Blackfriday to Add TODO list support for Blackfriday Jul 22, 2016
@bep bep added this to the v0.17 milestone Aug 5, 2016
@spf13 spf13 was assigned by bep Aug 8, 2016
bep added some commits Jul 21, 2016
@bep bep Add TODO list support for Blackfriday
Fixes #2269
283f120
@bep bep Add CSS class to TODO list and list items
See #2296
a7565fd
@bep bep Add a flag to turn task list support off
See #2296
da6844f
@bep bep Add tests for Blackfriday task lists
See #2296
19bfe3d
@bep bep Document Blackfriday task lists
See #2296
ab89dc1
@spf13 spf13 was unassigned by bep Sep 8, 2016
@bep
Collaborator
bep commented Sep 8, 2016

I have rebased this against master now and I suspect the tests will pass.

I'm planning to implement this for the MMark renderer, too, but I kind of need this code to do that.

@digitalcraftsman @moorereason I know this works, but do you have any complaints? Good to merge?

@moorereason
Collaborator

Any reason why you set checked and disabled to empty strings? I would use:

<input type="checkbox" checked disabled class="task-list-item">
@bep
Collaborator
bep commented Sep 8, 2016 edited

Any reason why you set checked and disabled to empty strings?

If you followed the looooong discussion in the related issue you will see that the core part of the implementation is borrowed from @shurcooL ... Knowing that he writes compilers for fun (gopherjs), I'm pretty sure he is correct. And reading the spec (https://www.w3.org/TR/html-markup/syntax.html#syntax-attributes), I don't see any "attribute without value" option. It probably works in most (all?) browsers, but isn't entirely correct.

@shurcooL
shurcooL commented Sep 8, 2016 edited

If you followed the looooong discussion in the related issue you will see that the core part of the implementation is borrowed from @shurcooL ... Knowing that he writes compilers for fun (gopherjs), I'm pretty sure he is correct.

Thanks, but I would recommend reviewing that code from scratch. I wrote it a while ago and decisions/research done back then may not apply or still be valid today. It also highly depends on the context where it's used (version of HTML, etc.).

Any reason why you set checked and disabled to empty strings?

There was no good reason, it was just one of many ways of doing it. I think your suggestion is valid and makes sense. But I haven't looked at the specifics of HTML/XHTML versions hugo uses, its browser support, etc.

@bep
Collaborator
bep commented Sep 8, 2016

Both variants is fine, then. I chose the one with the least amount of work.

@bep bep Simplify task list HTML attributes
`checked` and `disabled` doesn't have to have an empty value.

See #2296
0e47783
@bep
Collaborator
bep commented Sep 9, 2016

@moorereason I have simplified the HTML attributes re. your comment, does it then look generally OK?

@shurcooL if people use your GitHub renderer (or whatever it is named), you should take another look at your TODO-list implementation. A HTML list without any CSS classes isn't very easy to style.

@digitalcraftsman
Collaborator

The commit looks fine. I agree with @moorereason that the empty strings can be removed because the w3c spec handles them equivalent:

Note that empty attribute syntax is exactly equivalent to specifying the empty string as the value for the attribute, as in the following example.

<input disabled="">

@bep bep merged commit eaf2f9b into spf13:master Sep 9, 2016

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/wercker Wercker build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@bep bep deleted the bep:tastklist-bf branch Sep 9, 2016
@shurcooL
shurcooL commented Sep 10, 2016 edited

@shurcooL if people use your GitHub renderer (or whatever it is named), you should take another look at your TODO-list implementation. A HTML list without any CSS classes isn't very easy to style.

The goal of the github_flavored_markdown package is to render GitHub Flavored Markdown, generating HTML and providing the CSS that displays it. The user isn't meant to customize it in any way – that's what blackfriday and its custom renderers and options are for. If you think there's a bug, you should open an issue in that repo.

@bep
Collaborator
bep commented Sep 10, 2016

If you think there's a bug, you should open an issue in that repo.

I don't use that repo, just giving you a heads up.

@bwinterton bwinterton referenced this pull request in jpescador/hugo-future-imperfect Oct 26, 2016
Open

Add Markdown Todo list support for Hugo 0.17 #26

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