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

Add print.enableCheckBoxes config option #962

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

niekschoemaker
Copy link

@niekschoemaker niekschoemaker commented Jun 9, 2021

Fix #774

This option enables the checkboxes in the outputted HTML, useful for making to-do lists

This option enables the checkboxes in the outputted HTML, useful for making to-do lists
Auto-update on TOC removed the KBD tags from the title for some reason
@yzhang-gh
Copy link
Owner

Many thanks 👍.

Looks good to me. cc @Lemmingh

@Lemmingh Lemmingh added the Area: Configuration Requiring changes in configuration schema. label Jun 18, 2021
@Lemmingh Lemmingh marked this pull request as draft June 18, 2021 12:03
@Lemmingh
Copy link
Collaborator

Overall, neat.

Well, I believe it would be best to always keep the preview and exporting consistent, so linked #774 above.

Then, 2 concerns come:

  1. Now we have to duplicate code at two modules (extension and markdownEngine), which is a bit inconvenient. Is it possible to extract a new module, probably called markdown-it-plugins?

  2. print.enableCheckBoxes doesn't sound good. I tried to find some inspiration from Typora's preferences panel, but failed. My suggestion for now is tasklist.output.clickable. The words are picked from GFM and MDN.

@yzhang-gh
Copy link
Owner

Thanks.

Well, I believe it would be best to always keep the preview and exporting consistent, so linked #774 above.

I thought it was okay to disable the checkboxes as we really cannot check a to-do item within the preview pane (it won't change the source file). Now I was wondering whether we should enable it by default. I don't think there are many use cases that require the checkboxes to be disabled. And then we don't have to introduce this option at all.

@yzhang-gh yzhang-gh mentioned this pull request Jun 20, 2021
12 tasks
@Lemmingh
Copy link
Collaborator

we really cannot check a to-do item within the preview panel (it won't change the source file).

You mean #925?

Right. We cannot as long as the preview is provided by VS Code.

whether we should enable it by default.

I prefer to keep checkboxes disabled by default.

The output of a Markdown document is usually considered static. Few people expect it to be interactive.


Interestingly, those who would like an interactive output usually heavily take advantage of raw HTML and client-side script, and they can of course control the disabled attribute of any <input> element through DOM easily. Then, why bother to tweak Markdown renderer? 😂

I now don't feel the story (useful for making to-do lists) convincing.

@yzhang-gh
Copy link
Owner

The output of a Markdown document is usually considered static. Few people expect it to be interactive.

Agree. It can be kind of confusing to see an enabled checkbox which is in fact not working as one may expect.

@niekschoemaker Could you tell us more details about your use case?

@niekschoemaker
Copy link
Author

I'm using it to stay organized when doing certain tasks.
So for small checklists, same way as with a github issues. So for example a checklist of things to check before something is ready for release, i can simply tick of the checkboxes and leave the tab open, and since I already use markdown for a lot of documentation it just made sense to use it this way.

@yzhang-gh
Copy link
Owner

Thanks.

I also use checklists from time to time. However, I always make changes to the Markdown source file, as it can be saved for later. Just FYI, the keybinding Alt+C can be used to check/uncheck task list item(s).

As for this PR, let's also apply this option here

return md.use(require('markdown-it-task-lists'))

Then I think we can use the name tasklist.clickable? It affects both VS Code's preview and the exported HTML.

@Lemmingh
Copy link
Collaborator

I was thinking of a schema like:

/**
 * GitHub Flavored Markdown task list.
 * @see {@link https://github.github.com/gfm/#task-list-items-extension-}
 */
interface taskList {
    /**
     * Enable GFM task list.
     * @defaultValue `true`
     */
    enabled: boolean;

    /**
     * Control the editing experience.
     */
    editing: {
        /**
         * Allow the `markdown.extension.checkTaskList` command to turn a normal list item to a task list item.
         * @defaultValue `false`
         */
        aggressiveToggle: boolean;
    };

    /**
     * Control the rendering.
     */
    output: {
        /**
         * Render as clickable checkbox elements.
         * @defaultValue `false`
         */
        clickable: boolean;
    };
}

@yzhang-gh
Copy link
Owner

Well, I'm not a fan of tasklist.enabled and tasklist.editing.aggressiveToggle as I don't think they should even be false and true respectively.
The only reason for tasklist.enabled is someday VS Code may support task lists by itself, then we need something like math.enabled. But we don't have to include task lists in this repo anymore if it happens.
As for aggressiveToggle, I remember there is a feature request. But I don't think it is worth doing (partially because of my personal preference), at least until there are enough people who want it.

@Lemmingh
Copy link
Collaborator

We ought to reserve sufficient extensibility, even if we decide never to do something. Our product can focus on a small field, but our architecture cannot.

  • Markdown is a language with highly pluggable grammar. (so many flavors)

  • The "task list" we use is GFM task list.

  • Although CommonMark and its derivatives are popular nowadays, a large number of VS Code users still prefer to use flavors not compatible with CommonMark, such as Python-Markdown and kramdown.

  • Our code is licensed under the MIT License. Someone may create a fork for their own need.

Thus, it's necessary to build a friendly schema that allows modifying things with minimal effort.

Looks like we pay different attention to user experience. In #897 (comment), for instance, I asked about theming vs editor.theming vs editing.theming. Such subtle wording implies distinctly different organization, and can have a huge influence on extending system.


The only reason for tasklist.enabled is someday VS Code may support task lists by itself, then we need something like math.enabled. But we don't have to include task lists in this repo anymore if it happens.

I didn't mean to implement those in our code, but as stated above, we really need to take them into consideration when designing.

And I don't agree with your idea.

Coupling with vscode.markdown-language-features has brought us tons of trouble. We'd better stop doing that, and become a Markdown editor powered by VS Code. The best precedent I know is Azure Data Studio / mssql. I don't think we need to release a separate app. But I believe we should create an extension to VS Code, not vscode.markdown-language-features.


As for aggressiveToggle, I remember there is a feature request.

#842

@yzhang-gh
Copy link
Owner

Thus, it's necessary to build a friendly schema that allows modifying things with minimal effort.

I see the difference. You care more about developers' experience (i.e. the design) 😂, while I mainly follow my preference (of features), user voice, and then others.

Coupling with vscode.markdown-language-features has brought us tons of trouble. We'd better stop doing that, and become a Markdown editor powered by VS Code.

I was just thinking it would lift our burden if these basic features are implemented by the VS Code team (which is full-time). Then we don't have to spend our time repeating it. That is the original motivation.
But you may be right. If we want to provide a better user experience, this has to be "a Markdown editor powered by VS Code" rather than an extension. Although it obviously requires more effort 😅.

I wasn't that ambitious (as it may not be worth it, IMO). But at least I shouldn't refuse a suggestion that makes this work better.

Back to this PR, what do we do with the enabled and the aggressiveToggle given we are not going to implement them yet?

@Lemmingh
Copy link
Collaborator

what do we do with the enabled and the aggressiveToggle given we are not going to implement them yet?

If we can produce a reusable pattern here, we'll have a nice template when designing new parts of the configuration and refreshing existing hard-to-discover parts.

The draft schema above demonstrates a hierarchy:

  • Root

    • Definitive setting

    • Non-CommonMark syntax

      • Enabled
      • Variant selection
      • Editing
        • Fine-grained setting
      • Output
        • Fine-grained setting
    • CommonMark syntax

      • Editing
        • Fine-grained setting
      • Output
        • Fine-grained setting
    • Non-syntax-scoped feature

      • Fine-grained setting

Notes:

  • Settings are organized into three types of feature areas and "Definitive setting".

  • Definitive settings are to lock other settings. They are placed under the configuration root. I'm not sure whether we will need such things, but at least a workspace-scoped (resource) document.mode is very likely to come into being.

  • "Fine-grained setting" may contains sub areas and corresponding "Enabled".

  • For each "Non-CommonMark syntax":

    • "Enabled" refers to the enablement of the syntax, thus, is the switch of the whole section.
    • "Variant selection" is needed for things with many different variants, such as table and math.
  • For syntax-scoped settings:

    • "Editing" controls behavior within the Markdown editor, such as emphasis delimiter and list marker.
    • "Output" controls the rendering of exporting and preview. For now, it means how to convert Markdown to HTML.
  • For non-syntax-scoped features:

    • TOC, completion, editor theming, exporting, formatting, link handling, etc. fall into this category.
    • They are probable to have lots of sub areas.

@Lemmingh Lemmingh added the Area: Task list GitHub Flavored Markdown task list. label Aug 23, 2021
@goyalyashpal
Copy link

whst happened here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Configuration Requiring changes in configuration schema. Area: Task list GitHub Flavored Markdown task list.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to toggle the disabled attribute of checkbox in task list
4 participants