micromark / micromark-extension-gfm-task-list-item

micromark extension to support GFM task list items
https://unifiedjs.com
MIT License
6 stars 1 forks source link

HTML result has `disabled` attribute hard coded on the checkbox input #2

Closed brookback closed 7 months ago

brookback commented 7 months ago

Initial checklist

Affected packages and versions

micromark-extension-gfm-task-list-item@2.0.1

Link to runnable example

No response

Steps to reproduce

Process:

import parse from 'remark-parse';
import gfm from 'remark-gfm';
import toHTML from 'remark-html';

unified()
.use(parse)
.use(gfm)
.use(toHTML)
.process(`
- [] foo
`);

Expected behavior

<input type="checkbox">

Actual behavior

<input type="checkbox" disabled="">

Checkbox is disabled due to this hard coded line: https://github.com/micromark/micromark-extension-gfm-task-list-item/blob/main/dev/lib/html.js#L17

Maybe it can be configured somehow, to opt-out of the disabled default?

Runtime

Node v16

Package manager

No response

OS

macOS

Build and bundle tools

No response

wooorm commented 7 months ago

And then? You’d still need to do several other things to make an interactive form than saves things in a database. What’s your use case?

brookback commented 7 months ago

An addEventListener("input", handler) which takes care of the bubbled events on the checkboxes should be all I need :)

The use case is simply to use these rendered form elements. Right now, this library preventing any interaction with the elements.

The workaround is to let client side JS loop over the relevant elements and set el.disabled = false.

wooorm commented 7 months ago

If you can add event listeners, then you can also remove attributes indeed!

You should then also add names to each checkbox. And likely wrap in a form (or associate a form otherwise). Depends on where you want to store this info.

brookback commented 7 months ago

Sure. But may I ask why the default state is disabled: true?

ChristianMurphy commented 7 months ago

It is set in the standard https://github.github.com/gfm/#task-list-items-extension-

The reason behind that markdown is a static document. Input is interactive, it requires additional scripts, backend, and html context which isn't part of the markdown standard. So the default is disabled to make it static.

github-actions[bot] commented 7 months ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

brookback commented 7 months ago

It is set in the standard

Hm, no, it's not set. On the contrary, the spec literally has a paragraph saying that implementors are free to do whatever with the disabled prop:

This spec does not define how the checkbox elements are interacted with: in practice, implementors are free to render the checkboxes as disabled or inmutable elements, or they may dynamically handle dynamic interactions (i.e. checking, unchecking) in the final rendered document.

ChristianMurphy commented 7 months ago

Use your listening ears @brookback 😉 The standard offers two options. Micromark is not a full stack platform it is a static and immutable document rendered. So we offer the static and immutable disabled.

If you want to make it dynamic consider using one of our higher level packages like https://github.com/remarkjs/react-markdown Which allows you to customize this.

brookback commented 7 months ago

I didn't want to come off as pushy, sorry if so. Just asking since I want to understand the goals and reasoning, and I felt your initial comments weren't exhaustive.

Thanks so much for your time!

brookback commented 7 months ago

(my use case is that we use Remark on the server and pass the rendered HTML to the client. Thus, I don't use the React package linked. I still want client interactions on that rendered HTML though. So it's sort of an in between the chairs of the "immutable document" and "dynamic React component" worlds).

wooorm commented 7 months ago

If you use remark, then the HTML part of this project isn't used! Instead, these props are defined in mdast-util-to-hast.

With hast, there are trees, which could be changed.

But it all kinda depends on what you're doing. Should every user be able to modify markdown? Should their interactions be stored in a database to their account or in local storage? To do all of those things, the second option in the spec, you'd need more than just the disabled prop. Client side JS, or transforming the tree, adding more info like names and forms.

So I don't think it's sensible to do just that.

ChristianMurphy commented 7 months ago

my use case is that we use Remark on the server and pass the rendered HTML to the client. Thus, I don't use the React package linked.

The package linked can work server side, but lower level remark packages work too!

So it's sort of an in between the chairs of the "immutable document" and "dynamic React component" worlds

It's also possible with the lower level packages. remark focuses purely on markdown representation, but to render you're likely passing through rehype, which handles HMTL https://unifiedjs.com/learn/ rehype supports plugins, which allows defaults to be changed.

import type { Root } from 'hast';
import { visit } from 'unist-util-visit';

export function rehypeEnableCheckbox() {
  return function checkboxTransform(tree: Root) {
    visit(tree, 'element', (node) => {
      if (node.tagName === 'input' && node.properties.type === 'checkbox') {
        node.properties.disabled = false;
      }
    });
  };
}

runnable example https://stackblitz.com/edit/github-zqnym5?file=src%2Fmain.ts

brookback commented 7 months ago

@ChristianMurphy Huge thanks for the explanation! I use remark-html as a shortcut, but I see now how it's useful to drop down to the Rehype plugins separately :) I tried something like your transformer plugin, but gave up when setting disabled didn't work (due to me not using the plugin properly).

🙏🙏🙏