search

ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.65k stars 3.71k forks source link

Interactive checkboxes (a.k.a. GitHub TODO check list in Markdown) #1434

Closed zadam closed 5 years ago

zadam commented 5 years ago

Is this a bug report or feature request? (choose one)

🆕 Feature request

💻 Version of CKEditor

CKEditor 11.2.0

It's sometimes useful to be able to create an interactive checkbox for e.g. task lists etc. By that I mean you would be able to insert checkbox at the desired position by e.g. clicking on the toolbar and then you can check it and uncheck it in the editor.

Example

s1nceri7y commented 5 years ago

+ I think it would be great enhancement. Something similiar was done even in github, so we can use that syntax. https://blog.github.com/2014-04-28-task-lists-in-all-markdown-documents/

oleq commented 5 years ago

Hi! Thanks for letting us know that you'd welcome this kind of feature.

Anyway, what would you expect to see in your data? What the list items would look in Markdown and HTML when checked and unchecked?

cc @scofalik: What sort of engine problems do you reckon this feature would have to face? (e.g. do we need inline widgets for that?).

s1nceri7y commented 5 years ago

i think it would be great to have - [ ] if unchecked and - [x]if checked in lists. Check out github's example

`### Solar System Exploration, 1950s – 1960s

- [ ] Mercury
- [x] Venus
- [x] Earth (Orbit/Moon)
- [x] Mars
- [ ] Jupiter
- [ ] Saturn
- [ ] Uranus
- [ ] Neptune
- [ ] Comet Haley`

image

oleq commented 5 years ago
zadam commented 5 years ago

Well, previously I didn't necessarily view this to be connected to list at all and thought it might exist completely inline/separate as well. But on a second thought it's not clear to me what are the use cases outside of the list.

As for the HTML, I think about something like this:

<ul>
    <li><input type="checkbox" checked> First task</li>
    <li><input type="checkbox"> Second task</li>
</ul>

Not sure if there should be a label or not since the LI can be pretty long and it's not obvious what should be inside the label and what outside.

Also I can see a problem where the checkbox should be editable in the CKEditor itself, but should be probably disabled/effectively readonly in the HTML output, but again it's not clear to me how to do this.

For the UI questions - yeah, that's not obvious either. Maybe again thinking of this checkbox as totally separate insertable inline element (independent from list) could simplify this? So it would be just another item on the toolbar which would just insert input checkbox at the caret?

oleq commented 5 years ago

@Reinmar You may want to take a look at this proposal.

scofalik commented 5 years ago

We already tried to do something like this during a hackathon a year or two years ago. It was actually based on list feature implementation and it was even possible to make nested todo lists. I remember there were some problems with selection handling near checkboxes as those were created as UI elements.

Anyway, this should be doable, given that the PoC took a very little time and it was working nicely.

Reinmar commented 5 years ago

Do you think that we should implement todo lists as a separate attribute of a list or as a new list type?

What's the difference?

GitHub may not be the most representative and worth following examples (we should rather check what proper rich-text editors offer), but I checked that this:

1. [ ] Foo
2. [ ] Foo

Will get you this (no numbering):

  1. [ ] Foo
  2. [ ] Foo

However again, while for the user the above looks lidentical to this:

* [ ] Foo
* [ ] Foo

The markup produced by GitHub is different (there's <ol> instead of <ul>). However again, if the users don't see a difference, should we introduce two types of todo lists? Wouldn't that be confusing?

The answer isn't that simple because it depends on the styling. GitHub hides the bullets and numbers, so both types of lists looks alike. Should we also hide them?

The answer isn't that simple to me again. For one, we should check existing rich-text editors. However, I see a value in having numbered todo lists and bulleted todo lists. There's a difference between "buy these 4 things" and "do these 4 things in the right order".

ZMonk91 commented 5 years ago

@Reinmar I think you're overthinking the functionality here. Keeping them as a seperate list type makes more sense because a TODO-list is a seperate list. It has seperate use cases, with distinctively different goals and visual comprehension. As far as structuring a list to be ordered, there's no reason why the user can't simply move the line above or below. The value in keeping the syntax the same as Github's is in the universality of the feature.

Furthermore, a TODO-List can be specified as ordered or not depending on the context. For example:

Ingredients

versus

Recipe

Both can be expressed easily in a TODO-list without bullets or numbers. Both are understood to either be ordered or unordered depending on the context.

gritty-gitty commented 5 years ago

As long as I can remember, I’ve always been using such marks on my paper notes. Hence, this kind of checkboxes is a must-have in any note-taking app for me.

Some apps (like Zim, e.g.) go even a step further than GitHub by proposing 3 states for such a ToDo list:

  1. the unchecked state, marked with [ ] and displayed as (an empty) '☐';
  2. the completed state, marked with [*] and displayed as (a green) '☑';
  3. the discarded state, marked with [x] and discarded as (a red) '☒'.

That method does keep the record of any dead ends in the course of a project, at least.

dkonopka commented 5 years ago

So, here is the design proposal:

Regarding animations: I'm not a big fan of static UI elements. That's why you can see an animated strike as ::before pseudo-element (CSS limitations - we can't animate text-decoration: line-through). If we want to stay with this, the requirement is to wrap <li> content into <span> (supporting multiline).

Markup proposal (we can use checkbox as input[type="checkbox] instead of <span>)

<ul class="ck ck-todolist">
  <li class="ck-todolist__item">
    <span class="ck-todolist__item-checkbox"></span>
    <span class="ck-todolist__item-content">Create a strong device passcode</span>
  </li>
</ul>

Features: IMHO for the MVP, we should start with two states (but I need to admit 3rd state (discarded) sounds cool). There is also an idea to make single list item draggable.

Rectangle checkboxes

rectangle-todo

Circle checkboxes

cirlce-todo

Circle checkboxes look modern and fresh, but to be honest, rectangles fit better to the CKEditor UI. You can find codepen demo here to see mockup live.

Reinmar commented 5 years ago

@dkonopka, really cool animations. From the visual perspective, I don't have much to say.

But, regarding the output (and styling) we've got to talk about two things – the editor output and the editor content (data and editing views). We can make them different or identical, but have to consider similar aspects in both of those cases:

Regarding the styling itself:

pjasiun commented 5 years ago

But, regarding the output (and styling) we've got to talk about two things – the editor output and the editor content (data and editing views).

  • The markup outputted by the editor should use generic classes, no .ck stuff.

Agree. getData should not return any .ck stuff. However, we can use it in the editing pipeline. @oskarwrobel: can you propose something?

Nested lists. Nested lists with different types (e.g. a bulleted list inside a todo list).

I think that it should be supported from the beginning. You should be able to nest ordered, unordered and todo list any way you wants.

There was a discussion regarding whether you can have just a single type of todo list or ordered and unordered todo lists.

I think we should focus on unordered todo lists, for now. We should make it simple, provide 3 types of list type: ordered, unordered and unordered todo list. From the IU point of view, I think it should look like a third type of list (so you will have 3 buttons to choose the type of list). However, for the technical point of view (model representation and data returned by getData), we could implement it as an additional flag, so in the future, you will be able to have both ordered and unordered todo list. However, at this stage whenever you switch to todo list you will get unordered todo list. It should be both simple for now and open for extension in the future.

HTML semantics and accessibility.

I leave it to you, guys. However, when I use OneNote I very often use the keyboard to switch checkbox (checked/unchecked), so I think that some keyboard shortcut would be nice to have.

Regarding the styling itself:

I just realized that we also need an icon for that.

dkonopka commented 5 years ago

HTML semantics and accessibility. I'm not sure whether a checkmark can be represented easily with a single span. This raises huge accessibility concerns

I agree that's why we should use <input type="checkbox"> with possibility to focus and check/uncheck via Enter key (?).

<ul class="ck ck-todolist">
  <li>
    <input type="checkbox" checked>
    <span> ... </span>
  </li>
</ul>

This <span> might be cumbersome, but without it, we can't animate properly strike element and support it for multiline.

To me, those checkboxes are a bit too big (like 1-2px).

I tried, to make them smaller, but then icon inside checkbox is not similar to the checkmark. But we will be working on that during implementation.

Wouldn't a slightly bigger margin on the left make space for "drag handle" in the future?

👍

mlewand commented 5 years ago

Regarding ordered/unordered list types - question is how does one differentiate the type? GH choice doesn't make sense to me, as the list type is inaccessible to sighted users.

So I'd say starting with unordered seems to be reasonable. In case users miss ordered todo lists we'll get a proper feedback down the line.

This <span> might be cumbersome, but without it, we can't animate properly strike element and support it for multiline.

Regarding accessibility if we need to use spans for visuals, we can do that 👍 We'll just have to add extra aria markup to add proper semantics to this markup.

Proposed designs look really nice, I'd lean more toward rectangle boxes. As @dkonopka mentioned it looks more consistent with the v5 UI.

oleq commented 5 years ago

@dkonopka A slight "halo" around each checkbox on :hover like we already use in switch buttons maybe? (food for thought, not saying it's gonna look great)

Kapture 2019-07-23 at 8 51 15

vide https://ckeditor.com/docs/ckeditor5/latest/features/table.html

oleq commented 5 years ago

Another thing is the contrast (a11y). Make sure there's enough contrast between the white checkmark and the background. Could be the darker shade of green makes more sense.

pjasiun commented 5 years ago

I agree that's why we should use <input type="checkbox"> with possibility to focus and check/uncheck via Enter key (?).

I think it would be better to have some shortcut for checking this checkbox whenever the selection is in the list item. It would both simpler to use and to implement. Note that usually [Tab] is used to move the selection between items, and in lists [Tab] is also used to indent the item, so an intuitive flow might be complicated.

The question is: is there any common shortcut to check an item in the todo list? If no we can use some combination with the Enter key (for instance Ctrl+Shift+Enter)?

pjasiun commented 5 years ago

What do you think about using this in the data output (editor.getData):

<ul>
  <li>
    <label>
        <input type="checkbox" checked>
        Foo
    </label>
  </li>
</ul>

I nested input in label to avoid problem of using for attribute, which is problematic since we have no good way to create a nice ids (id could be generated based on the label content or we could use a random number or make it configurable but I do not like any of these solutions).

We can also ignore label and use:

<ul>
  <li>
        <input type="checkbox" checked>
        Foo
  </li>
</ul>

Or add a span mentioned by @dkonopka to make it easier for developers to style the output.

As for the editing view (what we will use in the editor), it does not matter that much. We may have there a custom .ck attributes and additional spans, if needed.

pjasiun commented 5 years ago

The question is: is there any common shortcut to check an item in the todo list? If no we can use some combination with the Enter key (for instance Ctrl+Shift+Enter)?

I was looking for such a shortcut and was not able to find any common solution, so I think we should propose our own and make sure it will have no conflict with other common applications shortcuts.

mlewand commented 5 years ago

I use OneNote I very often use the keyboard to switch checkbox (checked/unchecked), so I think that some keyboard shortcut would be nice to have.

I second to that the keystroke/hotkey would be a great productivity booster when using todo lists.

Unfortunately I'm not familiar with any app that do this, so I'd say we should research if there is some commonly used keystroke in these apps.

If there is no standard keystroke, we should find a key that is:

As such ctrl + space seems like a good option, though I don't know if it isn't reserved on any major Linux distros.

pjasiun commented 5 years ago

After a second thought, I think we should not use label. It makes the whole item clickable (clicking on it changes the state of the checkbox). Such label might contain link what means that when you misclick the link you check the checkbox. As @oskarwrobel also notice, one may want to select the list item text (especially if it is longer), and label might not allow you to do so easily.

So I think, we should keep the data output simple:

<ul>
  <li>
     <input type="checkbox" checked>
     Foo
  </li>
</ul>

As such ctrl + space seems like a good option, though I don't know if it isn't reserved on any major Linux distros.

Sounds good. It is fine on Mac (cmd+space is used by Spotlight, but ctrl + space looks to be available).

Reinmar commented 5 years ago

I second to that the keystroke/hotkey would be a great productivity booster when using todo lists.

What if we allow selecting the checkbox with the selection? Like an unremovable inline widget? Then, we could use simple Space.

Reinmar commented 5 years ago

Regarding the editor output – I think we need to answer the question whether such a content will be useful anywhere outside the editor. Or is it a content to be always used inside the editor?

I think the answer is – yes, you may want to create a post in something like Slack's editor and then share it with others.

But then:

jodator commented 5 years ago

From my POV (I'm using the Evernote's TODO lists) the use case is as such:

So I'm not sure if this use-case covers the above points.

pjasiun commented 5 years ago

Regarding the editor output – I think we need to answer the question whether such a content will be useful anywhere outside the editor. Or is it a content to be always used inside the editor? I think the answer is – yes, you may want to create a post in something like Slack's editor and then share it with others.

Agree. Then I propose something more similar to what @dkonopka proposed:

<ul class="todo-list">
  <li>
     <input type="checkbox" disabled checked>
     <span>Foo</span>
  </li>
</ul>

@dkonopka, @Reinmar WDYT?

What if we allow selecting the checkbox with the selection? Like an unremovable inline widget? Then, we could use simple Space.

I have mixed feeling about it.

Pros:

Cons:

oskarwrobel commented 5 years ago

We can change span to label without for attribute (it's correct from HTML POV).

<ul class="todo-list">
  <li>
     <input type="checkbox" disabled checked>
     <label>Foo</label>
  </li>
</ul>
pjasiun commented 5 years ago

I checked HTML spec and it looks like @oskarwrobel proposal is correct:

The label element represents a caption in a user interface. The caption can be associated with a specific form control, known as the label element's labeled control, either using the for attribute, or by putting the form control inside the label element itself.

Except where otherwise specified by the following rules, a label element has no labeled control. ... label.control - The control IDL attribute must return the label element's labeled control, if any, or null if there isn't one.

From: https://html.spec.whatwg.org/multipage/forms.html#the-label-element

jodator commented 5 years ago

proposal is correct:

Shouldn't it be 

<ul class="todo-list">
    <li>
    <label>
        <input type="checkbox" disabled checked>
        Foo
    </label>
    </li>
</ul>

as:

or by putting the form control inside the label element itself.

Reinmar commented 5 years ago

Agree with @jodator. <label> that is not associated with any input just doesn't seem to make sense. I may be wrong (cc @mlewand @Comandeer) but you could use a plain <span> and it would have the same meaning. And even if it makes any difference, it just doesn't make much sense.

oskarwrobel commented 5 years ago

For me, label looks better then span in this case if it's not incorrect I would go with a label, but span seems to be ok either.

Reinmar commented 5 years ago

PS. @pjasiun is right that a label doesn't have to be (according to the spect) associated with any input. But that's not my point. My point is – does it make sense to use a label here if we don't connect it to the input.

Comandeer commented 5 years ago

I don't see a point in using label next to the field and without connecting them. From a11y perspective it's incorrect and creates field without associated label.

@jodator proposal seems to be the best. It creates label for the checkbox and at the same time it does not force us to generate IDs.

oleq commented 5 years ago

I think it would be better to have some shortcut for checking this checkbox whenever the selection is in the list item. It would both simpler to use and to implement. Note that usually [Tab] is used to move the selection between items, and in lists [Tab] is also used to indent the item, so an intuitive flow might be complicated.

The question is: is there any common shortcut to check an item in the todo list? If no we can use some combination with the Enter key (for instance Ctrl+Shift+Enter)?

It's not MVP. We can release the feature without in–editor checkbox toggling and that's gonna be fine.

pjasiun commented 5 years ago

@jodator proposal seems to be the best. It creates label for the checkbox and at the same time it does not force us to generate IDs.

I prosed it 2 days ago and disagreed with myself. Let's stick to <span>, them.

What if we allow selecting the checkbox with the selection? Like an unremovable inline widget? Then, we could use simple Space.

I was thinking a little more about it and none of the solutions I know works this way, so since it is more tricky to implement I think that we should start from the solution where these items are transparent for the selection.

pjasiun commented 5 years ago

So...

The summary of the TODO list feature:

UI:

See UI proposal will be provided by @dkonopka: https://codepen.io/k0n0pka/pen/MNWjpO

Features:

Representation:

In data:

<ul class="todo-list">
  <li>
     <label class="todo-list__label [todo-list__label_checked]">
        <input class="todo-list__label__checkmark" type="checkbox" disabled checked />
        <span class="todo-list__label__description">Foo</span>  
     </label>
  </li>
</ul>

For nested lists:

<ul class="todo-list">
  <li>
     <label class="todo-list__label [todo-list__label_checked]">
        <input class="todo-list__label__checkmark" type="checkbox" disabled checked />
        <span class="todo-list__label__description">Foo</span>  
     </label>
      <ul class="todo-list">
        <li>
           <label class="todo-list__label [todo-list__label_checked]">
              <input class="todo-list__label__checkmark" type="checkbox" disabled checked />
              <span class="todo-list__label__description">Bar</span>  
           </label>
        </li>
      </ul>
  </li>
</ul>

In the editor:

We may use any additional markup needed for styling.

In the editing view:

Checkboxes will be represented as UI elements, transparent for the selection.

In the model:

~It will be represented as a separate attribute, so in the future, we will be able to support both ordered todo list, and unordered todo list. For now, the list type should automatically change to "bulleted" when todo list attribute is added.~ (edit: with @oskarwrobel we realized a number of problems related to this implementation. For instance, the bullet list toolbar icon will be by default active for todo lists, since it knows nothing about additional attribute)

Todo lists will be define in model as a separate list type:

<listItem listType="todo" todoListChecked="true">foo</listItem>
<listItem listType="todo">bar</listItem>

Note that, it does not prevent us from implementing "orderedTodo" type in the future if we will implement such lists. We will be also able to provide any UI on top of it if needed.

Other:

Reinmar commented 5 years ago 
<listItem listType="todo" checked="true">foo</listItem>

checked -> todoListChecked to avoid conflicts with other checked attributes (for the same reason why we have listType and not type).

<ul class="todo-list">
  <li>
     <input type="checkbox" disabled checked>
     <span>Foo</span>
  </li>
</ul>

That span without any class look bad to me – how will you be styling it or processing in any other way? I think it should have a class and this class should be following BEM.

Possibly, the same applies to the input. To avoid styling it via .todo-list > li > input, a class would be good.

pjasiun commented 5 years ago

That span without any class look bad to me – how will you be styling it or processing in any other way? I think it should have a class and this class should be following BEM.

Possibly, the same applies to the input. To avoid styling it via .todo-list > li > input, a class would be good.

Can you propose something?

pjasiun commented 5 years ago

checked -> todoListChecked to avoid conflicts with other checked attributes (for the same reason why we have listType and not type).

Already updated.

Comandeer commented 5 years ago

Let's stick to <span>, them.

From a11y perspective it's just a worse solution. If we're going to use span, I propose associating it with checkbox using [aria-labelledby]:

<ul class="todo-list">
  <li>
     <input type="checkbox" disabled checked aria-labelledby="checkbox-label1">
     <span id="checkbox-label1">Foo</span>
  </li>
</ul>

Alternatively we can disable label clickability while the content is being edited (via e.g. pointer-events: none). Outside the editor the default behaviour of label is rather a feature, not an issue.

Reinmar commented 5 years ago

Re: https://github.com/ckeditor/ckeditor5/issues/1434#issuecomment-515098199

Can you propose something?

cc @oleq.

Edit: In the next comment I proposed some classes but I need someone verifying this.

Reinmar commented 5 years ago

Data view

I'd like to understand one thing – why can't, in the data, the whole input be nested inside a <label>? Is that because of styling concerns (that you can't strike-trough just the text and not the checkmark too)? If so – can't we just have it like this:

<ul class="todo-list">
  <li>
     <label>
        <input type="checkbox" disabled checked  class="todo-list__checkmark">
        <span class="todo-list__label">Foo</span>  
     </label>
  </li>
</ul>

We've got the label for a11y. We don't need ids. We have spans for styling. And we have classes for easier and more precise targetting from stylesheets and other forms of processing.

Editing view

In the editing view I think we don't need a <label>. We can use spans and aria-labelledby or actually something totally custom that will be read well. We can also postpone working on this as a11y here is not part of the MVP. Since this is the editing view, we can modify it in the future to our needs.

dkonopka commented 5 years ago

Is that because of styling concerns (that you can't strike-trough just the text and not the checkmark too)?

With this markup, we will need an extra .checked class on <label> to style pseudo-checkbox properly (because by default input[type="checkbox"] will be hidden.

Other than that it should work properly. You can see it here: https://codepen.io/k0n0pka/pen/MNWjpO

Reinmar commented 5 years ago

Perhaps <li>s, in general, could have some better classes. E.g. class="todo-list__item todo-list__item-checked"?

oleq commented 5 years ago

It could look as follows:

<ul class="todo-list">
    <li>
        <label class="todo-list__item">
            <input class="todo-list__item__checkmark" type="checkbox" />
            <span class="todo-list__item__label">Set up a mobile carrier PIN </span>
        </label>
    </li>

    <li>
        <label class="todo-list__item todo-list__item_checked">
            <input class="todo-list__item__checkmark" type="checkbox"  checked />
            <span class="todo-list__item__label">Freeze your credit (US)</span>
        </label>
    </li>
</ul>

Although it's pretty weird seeing label.todo-list__item or label....._checked.

Edit: This made me think the markup is wrong. If you can't name it so it makes sense, something else is wrong. <label> should be a label, not a wrapper (even if HTML allows that). OFC, we can go with that but in the long run, it does not feel natural.

Maybe

<label class="todo-list__label [todo-list__label_checked]">
    <input class="todo-list__label__checkmark" type="checkbox" />
    <span class="todo-list__label__description">Set up a mobile carrier PIN </span>
</label>

?

pjasiun commented 5 years ago

I'd like to understand one thing – why can't, in the data, the whole input be nested inside a <label>? Is that because of styling concerns (that you can't strike-trough just the text and not the checkmark too)?

As I already mentioned some time ago:

After a second thought, I think we should not use the label. It makes the whole item clickable (clicking on it changes the state of the checkbox). Such label might contain link what means that when you misclick the link you check the checkbox. As @oskarwrobel also notice, one may want to select the list item text (especially if it is longer), and label might not allow you to do so easily.

Such label also means that the return data works differently than the one you have in the editor. In the editor you need to click on the checkbox to change the value. Outside the editor, you can also click on the label. It could be misleading if the integration allows you to dynamically switch between the editor and the content.

I am also not sure if it will be correct for nested lists:

<ul class="todo-list">
  <li>
     <label>
        <input type="checkbox" disabled checked  class="todo-list__checkmark">
        <span class="todo-list__label">Foo</span>  
        <ul class="todo-list">
          <li>
             <label>
                <input type="checkbox" disabled checked  class="todo-list__checkmark">
                <span class="todo-list__label">Bar</span>  
             </label>
          </li>
        </ul>
     </label>
  </li>
</ul>

For such case we would need to transform it to:

<ul class="todo-list">
  <li>
     <label>
        <input type="checkbox" disabled checked  class="todo-list__checkmark">
        <span class="todo-list__label">Foo</span>  
     </label>
      <ul class="todo-list">
        <li>
           <label>
              <input type="checkbox" disabled checked  class="todo-list__checkmark">
              <span class="todo-list__label">Bar</span>  
           </label>
        </li>
      </ul>
  </li>
</ul>

What might be tricky to implement.

pjasiun commented 5 years ago

In the editing view I think we don't need a <label>. We can use spans and aria-labelledby or actually something totally custom that will be read well. We can also postpone working on this as a11y here is not part of the MVP. Since this is the editing view, we can modify it in the future to our needs.

I think that we can handle a11y for todo lists later.

dkonopka commented 5 years ago

So to sum up the design topic: you will find here https://codepen.io/k0n0pka/pen/MNWjpO a finished proposal with an icon of the to-do list.

pjasiun commented 5 years ago

@wwalc said that <label> is necessary in the data view in MVP, so we will code it this way. I will update the summary.

pjasiun commented 5 years ago

EDIT: You can find the demo of to-do lists here: https://ckeditor.com/docs/ckeditor5/latest/features/todo-lists.html

I am pleased to inform you that to-do lists, developed by @oskarwrobel just landed on the master branch and will be available in the next public release!

todo-list