Add option to let the user check/uncheck the checkboxes #249

g0rk4 · 2018-02-27T17:00:42Z

No description provided.

facelessuser · 2018-02-27T17:03:51Z

I'm not sure I follow what this pull is trying to solve, can you explain?

facelessuser · 2018-02-27T17:20:23Z

Okay, let me just give some feedback. In general, this is used to create static pages, but I get the feeling you want to do something more dynamic. Why not use JavaScript? Can you elaborate why you think this is needed?
Tests are currently failing, so that would need to be fixed assuming I decide to accept this.
Documentation would need to be updated to describe the new feature (again assuming I decide to accept this).
Add test(s) to test new feature when enabled.

g0rk4 · 2018-02-27T19:20:09Z

Hi @facelessuser,

First of all thank you for your quick reply !

Indeed I need this functionnality in order to be able to check dynamicaly some checkboxes. For exemple, I write some documentations of installation tools and I want to let the final user to follow the procedure by checking the steps already done.

If you will accept this pull request, I'll fix all the things you mentioned :
1 - Fix tests failure;
2 - Update documentation;
3 - Add new test(s) case;

facelessuser · 2018-02-28T00:45:07Z

I guess I'm okay with this, but all the mentioned things will have to be addressed first. New feature should be tested, all tests must pass, and there needs to be documentation.

g0rk4 · 2018-02-28T11:09:21Z

Hi @facelessuser , i made changes on PR :

Add option to let the user check/uncheck the checkboxes;
Test cases added;
Update documentation;

And now i'll try to understand why the tests are falling on your CI

facelessuser

Please fix all noted comments.

facelessuser · 2018-02-28T14:11:42Z

docs/src/markdown/extensions/tasklist.md

@@ -85,7 +99,7 @@ Classes               | Description
    }
    ```

-??? settings "Custom Tasklist CSS"


This is a details object, not a header. Please change the syntax back to how it was.

facelessuser · 2018-02-28T14:12:24Z

docs/src/markdown/extensions/tasklist.md

+| `task-list-control`   | This is attached to the `label` tag and represents the control object.                                 |
+| `task-list-indicator` | This is attached to the `span` directly following the input and is used to style the visual indicator. |
+
+### settings "Basic Tasklist CSS"


This should be a details object, not a header. Please change the syntax to how it was.

facelessuser · 2018-02-28T14:13:23Z

docs/src/markdown/extensions/tasklist.md

-Option            | Type | Default      | Description
----------------- | ---- | ------------ | ------------
-`custom_checkbox` | bool | `#!py3 False` | Generate task lists in such a way as to allow for styling the check box with CSS.
+| Option               | Type | Default       | Description                                                                       |


Please don't rewrite table syntax that I am using. Please remove leading and trailing pipes (|). I understand some may prefer to have tables flanked by those, but I do not.

facelessuser · 2018-02-28T14:17:10Z

pymdownx/tasklist.py

    """Get checkbox tag."""

    if custom_checkbox:
        return (
            '<label class="task-list-control">' +
-            '<input type="checkbox" disabled%s/>' % (' checked' if state.lower() == 'x' else '') +
+            '<input type="checkbox"%s%s/>' % (' disabled' if clickable_checkbox == False else '',


Tests are failing because you added a trailing self closing / in the input tag. This is probably better so that when people insert this into xhtml (but who is doing this these days). So I'm fine with the change, but you need to regenerate changed, failing tasklist tests.

facelessuser · 2018-02-28T14:18:47Z

I've noted in the review why CI is failing. Also, I've noted additional changes that need to fixed.

g0rk4 · 2018-02-28T15:42:18Z

Sounds good, thank you @facelessuser.

Tell me if something else is needed

facelessuser · 2018-02-28T16:05:04Z

.vscode/settings.json

@@ -0,0 +1,5 @@
+{


Please remove .vscode/settings.json file. Feel free to exclude it from the repo via .gitignore.

Done, i'll push changes when we wil be ok with the /

facelessuser · 2018-02-28T16:07:34Z

There was actually no need to remove the / in tags. As I mentioned it is better for xhtml. I was more mentioning we needed to regen existing tests not new tests. But maybe we can just do that as a separate issue.

g0rk4 · 2018-03-01T08:23:17Z

Hi @facelessuser,

I don't understand about the / because I do not removed any one in tags.

facelessuser · 2018-03-01T14:22:17Z

I was referring to the fact that you changed to '<input type="checkbox"%s%s/> ', and then back to '<input type="checkbox"%s%s> '. /> is more appropriate as this could be used in xhtml. I cannot look at the history because you force pushed, which rewrote the history, but that was originally why it was failing.

For custom tasklists it is still setting />, but for normal tasklists, it is not. It should be the same for both. I would prefer it to be the same for both. That is what I was saying. Technically, this is a bug on master, but you fixed it, and then reverted it. I was mentioning earlier that it was good that you fixed it, and you just needed to regenerate the failing tests to fix the CI failures.

g0rk4 · 2018-03-01T14:53:53Z

All right, now I get it !

I'll fix it this evenning

g0rk4 · 2018-03-01T15:36:27Z

Ok, it's done

facelessuser · 2018-03-01T15:51:39Z

Thanks, I'll try this out some time soon, but so far, everything looks good.

g0rk4 · 2018-03-01T15:53:13Z

My pleasure

facelessuser · 2018-03-01T15:58:54Z

Went ahead and downloaded it. Everything looks in order. This will be in 4.9.0.

facelessuser added Insufficient Details labels Feb 27, 2018

facelessuser removed the Insufficient Details label Feb 28, 2018

g0rk4 force-pushed the add-clickable-checkbox branch from b133b02 to c58fca2 Compare February 28, 2018 11:00

facelessuser requested changes Feb 28, 2018

View reviewed changes

g0rk4 force-pushed the add-clickable-checkbox branch 3 times, most recently from 813c170 to 9a07c5c Compare February 28, 2018 15:29

facelessuser requested changes Feb 28, 2018

View reviewed changes

g0rk4 force-pushed the add-clickable-checkbox branch from 9a07c5c to 806c82e Compare March 1, 2018 15:14

Add option to let the user check/uncheck the checkboxes

00f503d

g0rk4 force-pushed the add-clickable-checkbox branch from 806c82e to 00f503d Compare March 1, 2018 15:27

facelessuser added this to the 4.9.0 milestone Mar 1, 2018

facelessuser added In Progress and removed Needs Update labels Mar 1, 2018

facelessuser approved these changes Mar 1, 2018

View reviewed changes

facelessuser merged commit 1b5d8e5 into facelessuser:master Mar 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to let the user check/uncheck the checkboxes #249

Add option to let the user check/uncheck the checkboxes #249

g0rk4 commented Feb 27, 2018

facelessuser commented Feb 27, 2018

facelessuser commented Feb 27, 2018 •

edited

Loading

g0rk4 commented Feb 27, 2018 •

edited

Loading

facelessuser commented Feb 28, 2018

g0rk4 commented Feb 28, 2018

facelessuser left a comment

facelessuser Feb 28, 2018

facelessuser Feb 28, 2018

facelessuser Feb 28, 2018

facelessuser Feb 28, 2018

facelessuser commented Feb 28, 2018

g0rk4 commented Feb 28, 2018

facelessuser Feb 28, 2018

g0rk4 Mar 1, 2018

facelessuser commented Feb 28, 2018

g0rk4 commented Mar 1, 2018

facelessuser commented Mar 1, 2018 •

edited

Loading

g0rk4 commented Mar 1, 2018

g0rk4 commented Mar 1, 2018

facelessuser commented Mar 1, 2018

g0rk4 commented Mar 1, 2018

facelessuser commented Mar 1, 2018

Add option to let the user check/uncheck the checkboxes #249

Add option to let the user check/uncheck the checkboxes #249

Conversation

g0rk4 commented Feb 27, 2018

facelessuser commented Feb 27, 2018

facelessuser commented Feb 27, 2018 • edited Loading

g0rk4 commented Feb 27, 2018 • edited Loading

facelessuser commented Feb 28, 2018

g0rk4 commented Feb 28, 2018

facelessuser left a comment

Choose a reason for hiding this comment

facelessuser Feb 28, 2018

Choose a reason for hiding this comment

facelessuser Feb 28, 2018

Choose a reason for hiding this comment

facelessuser Feb 28, 2018

Choose a reason for hiding this comment

facelessuser Feb 28, 2018

Choose a reason for hiding this comment

facelessuser commented Feb 28, 2018

g0rk4 commented Feb 28, 2018

facelessuser Feb 28, 2018

Choose a reason for hiding this comment

g0rk4 Mar 1, 2018

Choose a reason for hiding this comment

facelessuser commented Feb 28, 2018

g0rk4 commented Mar 1, 2018

facelessuser commented Mar 1, 2018 • edited Loading

g0rk4 commented Mar 1, 2018

g0rk4 commented Mar 1, 2018

facelessuser commented Mar 1, 2018

g0rk4 commented Mar 1, 2018

facelessuser commented Mar 1, 2018

facelessuser commented Feb 27, 2018 •

edited

Loading

g0rk4 commented Feb 27, 2018 •

edited

Loading

facelessuser commented Mar 1, 2018 •

edited

Loading