-
-
Notifications
You must be signed in to change notification settings - Fork 194
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
TaskList: Listener to update markdown #511
Comments
I was thinking on high-level implementation would be something like:
|
I don't think IDs are necessary. In fact, GH doesn't use IDs at all. Instead, they base it on the order of first appearance/detection. I used the following markdown to test what was being sent back and forth from the browser to GH's servers: Cras justo odio, dapibus ac facilisis in, egestas eget quam. Duis mollis, est non commodo luctus, nisi erat porttitor ligula, eget lacinia odio sem nec elit. Duis mollis, est non commodo luctus, nisi erat porttitor ligula, eget lacinia odio sem nec elit.
Task List 0
- [ ] Item 0
- [x] Item 1
- [ ] Item 2
Curabitur blandit tempus porttitor. Aenean lacinia bibendum nulla sed consectetur. Nullam quis risus eget urna mollis ornare vel eu leo.
Task List 1
- [ ] Item 0
- [x] Item 1
- [ ] Item 2
Maecenas faucibus mollis interdum. Vestibulum id ligula porta felis euismod semper. Donec id elit non mi porta gravida at eget metus. When dragging
You can see that the source ( How they actually perform the modification is anyone's guess, but I suspect they have a strict AST of their markdown which allows them to more easily move these items around at a whim. As far as their FE sides of things, they implement their own This all being said, regardless of how this is implemented (which could be any number of ways), this is ultimately a specific server/application implementation. I'm not entirely sure how this project can provide "documentation" for something that would be specifically unique to each implementation. |
I forgot to mention that the "checked" data looked like this:
|
From discussing this with @colinodell in slack, I think adding a However, after thinking about this more over the weekend I realize that this would only alter the final HTML output. It wouldn't be able to reconstruct the markdown, which would likely be a requirement, especially if this is in some sort of CMS like application that has to save the original markdown that was modified. This means that some components from https://github.com/thephpleague/html-to-markdown may be needed here. Also, I'm not entirely sure how the "operations" ^^ should/would be queued. In theory, yes, we could easily use (abuse) the config since this extension doesn't have any; that, however, feels very wrong. These are (one time) operations/actions, not static configuration. I honestly don't think there is any elegant solution here (yet). We likely won't know more about how to properly tackle this until, at a minimum, 2.0 is released. Even then, we'd also likely need to standardize the AST more and probably introduce components of https://github.com/thephpleague/html-to-markdown to effectively perform the necessary operations on the source. |
Leaving aside how GitHub has implemented, I'm fairly certain that doesn't need to use an HTML converter if each checkbox has a unique id and there's a way to serialize AST back to markdown. then the unique id that's ticked gets send over the wire, you find respective checkbox element from AST, apply state change and that's it. I haven't seen the AST in this project, so I might be overoptimistic due that :) |
Considering that the
There's currently no reliable way AFAIK to convert the AST back into markdown (in this project specifically). That's part of the "components" I was referring to from https://github.com/thephpleague/html-to-markdown. We won't need to parse HTML during a
This "unique id" would work for this particular operation, but it wouldn't be able to handle the movement of items inside an existing list or between lists. This is something we'd likely want to support as well since it too is a very useful feature. Building on top of the first reply, GH has already determined that a simple two-point index-based coordinate system is sufficient. This allows it to:
I think the second point is likely the main reason they don't use identifiers as any identifier generated would likely be a result of the current index, which can change upon an item being moved; thus only causing confusion when the original identifier would be referencing a different item. |
This is related to #419 as both would require rendering the AST to Markdown |
This is kind of off topic as I'm not concerned about updating the source markdown but this is how I dealt with persisting the checkbox states in lieu of any official method. document.querySelectorAll('input[type="checkbox"][disabled]').forEach((checkbox) => {
checkbox.disabled = false; // enable checkbox (by default they are disabled)
checkbox.id = btoa(checkbox.parentElement.textContent.trim()); // generate a unique id
checkbox.checked = JSON.parse(localStorage.getItem(checkbox.id)); // set status of checkbox based on Local Storage value
checkbox.addEventListener('change', (event) => { // store checkbox state when it is changed
localStorage.setItem(event.target.id, event.target.checked);
});
}); IDs will change if the text is changed but that's kind of what I want so you might have to fiddle around if you don't like this. You could identify checkboxes based on another method, like index position for instance as mentioned. |
I was wondering if commonmark provided example for TaskListExtension how to:
this would make implementing full task list functionality easier for downstream projects.
The text was updated successfully, but these errors were encountered: