Skip to content
This repository has been archived by the owner on Oct 19, 2022. It is now read-only.

Tasks using hand coded html don't behave as expected #31

Open
digitalmoksha opened this issue Dec 18, 2019 · 7 comments
Open

Tasks using hand coded html don't behave as expected #31

digitalmoksha opened this issue Dec 18, 2019 · 7 comments
Labels

Comments

@digitalmoksha
Copy link

Tasks that are created using hand coded html don't properly work. For example

<ul>
<li>[ ] one</li>
</ul>

- [ ] two

gives a task item with the proper classes, such as task-list-item-checkbox, but it won't behave correctly because there is no underlying markdown task. In the above example, if you click task one, task two will get checked.

This doesn't happen often, but we've had issues where someone tries to use a task in a table, for example.

What do you think about somehow leaving these types of checkboxes disabled all the time, but still styled correctly so that they look like tasks?

If you know that your markdown has sourcepos support, then the lack of that attribute would indicate there is no markdown backing it up. But I'm not sure how to do it generically.

Of course the ideal thing would to be able support non-markdown checkboxes, but I'm not sure I see that happening yet 😄

@deckar01
Copy link
Owner

I'm not sure this was an intended use case, but HTML lists are valid markdown, so I don't think we should explicitly disable this syntax. Since the backend can't differentiate between the two use cases, I think the frontend needs to support this use case. The itemPattern should just need <\s*li\s*[^>]*> added as a prefix alongside the markdown list item patterns.

@deckar01 deckar01 added the bug label Dec 19, 2019
@deckar01
Copy link
Owner

deckar01 commented Dec 19, 2019

It wouldn't be at the start of the line though, so it would also need to match anywhere in the line. Also, in the table use case, it is a markdown table, so there could theoretically be multiple checkboxes on a single line... I think we would want to punt this to using a real markdown parser on the front end rather than making the regex parser more complex.

For sourcepos, we could try to find the nearest parent that has a sourcepos to at least identify the line number. That would work for simple use cases. I don't the markdown filter has enough info to accurately insert its own sourcepos attributes for inline offsets, which would be needed to handle multiple checkboxes on one line.

@deckar01
Copy link
Owner

deckar01 commented Dec 20, 2019

GitHub still has this problem even though they update the markdown on the backend, so this might not even be fixable with a markdown parser on the front end.

@deckar01 deckar01 removed the wontfix label Dec 20, 2019
@deckar01
Copy link
Owner

I'm taking off "won't fix". It would be possible to scan lines looking for HTML list items. If someone wanted to do this I would review the code and consider it. I don't think it is terribly important, but it might not be too complex to maintain.

@digitalmoksha
Copy link
Author

@deckar01 sorry I was slow to respond. Totally agree about the table case, that's a much deeper problem.

I think that if a checkbox can't actually be checked and have it's state changed, then I don't think it should be an active checkbox. Otherwise it just confuses the user, "why isn't the state being saved". I think rendering it as a checkbox is good, it just shouldn't be clickable.

I think that is doable. I'll look into it some more when I have a chance.

@deckar01
Copy link
Owner

If you know that your markdown has sourcepos support, then the lack of that attribute would indicate there is no markdown backing it up.

This may be true now, but I wouldn't rely on this. HTML is valid markdown and the sourcepos behavior is under-documented. It would be safe to assume HTML sourcepos could land in any patch release of commonmark.

The spec allows sourcepos on anything including HTML.

https://github.com/commonmark/commonmark-spec/blob/cb136a48ca97e3eb4dde7df775ebfe2cfcf087dd/CommonMark.dtd#L83-L90

The cmark docs makes it sound like a bug that some block elements aren't being annotated.

https://hexdocs.pm/cmark/Cmark.html#to_html/2

@digitalmoksha
Copy link
Author

☝️ Ah, good point...hmm...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants