Release 4.8 breaks task list styling

[jhildenbiddle]

Notice the missing "task-list-item" class on the <li> elements.

Markdown:

- [x] Task 1
- [ ] Task 2
- [ ] Task 3

Docsify 4.7.x Output:

<ul>
  <li class="task-list-item"><input type="checkbox" checked="">Task 1</li>
  <li class="task-list-item"><input type="checkbox">Task 2
  <li class="task-list-item"><input type="checkbox">Task 3
</ul>

Docsify 4.8.x Output:

<ul>
  <li><input checked="" disabled="" type="checkbox"> Task 1</li>
  <li><input disabled="" type="checkbox"> Task 2
  <li><input disabled="" type="checkbox"> Task 3
</ul>

Necessary code was deleted here (lines 299-310): a39b214#diff-d73f215ac772d2673dd8c51f24976e4eL299

[timaschew]

We should add some test cases. I saw similar problems, for instance: #717

[timaschew]

I guess you rely on the class?

I think the changes was an improvement (but also a breaking change).
Because the checkbox were not disabled. The logic to parse this was removed from docsify because the marked parser can handle it now.

The question is if it's worth tot add the code again in order to attach the class Instead of handle this as a breaking change. In future docsify should bump its major version for changes like this.

@jhildenbiddle what do you think?

[jhildenbiddle]

Hi @timaschew,

The change presents a few issues:

1. The class is the only hook for theme authors to style task lists.

2. The class is the only hook for plugin authors to efficiently target tasks lists on the client.

3. Task lists are incorrectly rendered as bullet lists (with task items) instead of task lists.

   GitHub

   • Task 1
   • Task 2
     • Subtask A
     • Subtask B
   • Task 3

   docsify 4.7
   

   docsify 4.8
   

I think it's debatable whether disabling checkboxes is an improvement or not. It makes sense given that task lists do not maintain state, but it also significantly reduces the visibility of the checkboxes (which is really an issue with the default styling on certain browsers, not docsify).

My preferred solution to all of this would be as follows:

1. Apply a "task list" class to the parent <ul>. This makes it easy to target both the parent list element and its children.
2. Replace each task item <input> element with an inline <svg> element. This allows portions of the image to be styled via CSS (e.g. making the check match the theme color) and prevent users from toggling the checkbox state.
3. Modify the HTML output to make task lists accessible by wrapping task items in a <label> element (easiest) or assigning the checkbox role to the SVG element along with the necessary for and id attributes.

I'd be happy to make these changes, but since I received no response from #603 it's been hard to prioritize docsify contributions.

Finally, the version bump. I don't think this type of change necessitates a major version bump since it isn't really "breaking" if it is the intention of the docsify maintainers to render tasks lists differently. A minor version bump with a note in the CHANGELOG seems appropriate from an end-user perspective, but it does highlight the need to keep plugin/theme developers informed. This is why I filed an issue (#552) asking for these kinds of changes to be included in the CHANGELOG.md. Unfortunately, I haven't seen this happen even though the issue has been closed, so I'm forced to periodically check to see if a new versions of docsify has been released and then review both the docsify-themeable and docsify-tabs sites to see if anything has changed. To be honest, it's a less-than-ideal developer experience that should be easy to address.

[timaschew]

Hi @jhildenbiddle

alright I see the use case for it. I was actually expecting to achieve it much more simpler by using a selector like this: .docsify-content li>input. But that doesn't solve the problem to identify the parent <ul>.

Replacing the input with a SVG sounds a bit overkill to me to be honest. Can't you style the input as well, like this custom checkbox? Anyway, I would suggest to fix the styling first and then we can discuss about the SVG in a dedicated pull request.

Providing a <label> makes totally sense IMHO.

I'd be happy to make these changes, but since I received no response from #603 it's been hard to prioritize docsify contributions.

It seems that he is really busy. But I would say let's just try. If he has no time we (the rest) need to agree at least ;)

So IMHO you can go ahead to make a PR which solves these both issues:

1. Apply a "task list" class to the parent <ul>. This makes it easy to target both the parent list element and its children.

3. Modify the HTML output to make task lists accessible by wrapping task items in a element (easiest) or assigning the checkbox role to the SVG element along with the necessary for and id attributes.

In the meantime I will try to setup some tests, which will run on Travis CI.

---

Finally, the version bump. I don't think this type of change necessitates a major version bump since it isn't really "breaking" if it is the intention of the docsify maintainers to render tasks lists differently

Sure API-wise it's not a breaking change. But on the visual layer it's a breaking one IMHO.

I totally agree with you to put changes like this (and actually everything into the CHANGELOG.md).
I'm pretty sure we will achieve this.

[timaschew]

I wonder if it makes more sense to extend the markdown parser instead of adding the logic to docsify: https://marked.js.org/#/USING_PRO.md
We just adjust with @QingWei-Li because he has on the 5.0 roadmap this item:

New Markdown Syntax: GitHub(or GitLab) Flavored Markdown ref

which probably means that marked is no longer used for parsing markdown?

[jhildenbiddle]

Thanks for the quick reply, @timaschew.

The inline <svg> suggestion was made to address issues with styling checkboxes in a way that will work with all supported browsers. The custom checkbox demo is interesting, but it doesn't work in all browsers (most notably IE and Edge, but also older versions of Firefox). Technically the demo shouldn't work in any browsers because pseudo-content should only work on container elements. From MDN web docs:

Note: The pseudo-elements generated by ::before and ::after are contained by the element's formatting box, and thus don't apply to replaced elements such as <img>, or to <br> elements.

An <input> element is a replaced element and not a container element, so pseudo content shouldn't render. I'm guessing the -webkit-appearance declaration makes this possible and that Firefox decided to update their behavior to match (it only works in Firefox 63+). Given the issues in IE and Edge, styling the <input> directly is a non-option if we want the presentation to be consistent across all supported browsers. Inline <svg> elements are just one solution. Another is to accept that IE and Edge task lists will look different, knowing that these engines are either changing (Edge => Chrome engine) or will no longer warrant support in the future (IE10/11). I'll investigate options and share my $0.02 on what seems to work best.

I agree that extending the markdown parser would be the right way to implement this. I haven't noodled with the docsify source enough to know how to implement this and still maintain the end-user's ability to extend the markdown parser as well.