-
Notifications
You must be signed in to change notification settings - Fork 6
Introduced todo lists. #133
Conversation
Result:
|
Fixed. |
Fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me 👍
During testing a collaboration I've noticed that shortcut for checking list on OS X triggers changing the language. We should consider if Ctrl + Space is a good idea. cc @pjasiun @oskarwrobel Edit: It turned out that I had enabled some system shortcuts which aren't enabled by default. After talking with @pjasiun, we decided to leave it as it is. |
Feature: Introduced to-do lists. Closes ckeditor/ckeditor5#1434.
|
||
writer.setAttribute( 'listType', 'todo', modelItem ); | ||
|
||
if ( data.viewItem.getAttribute( 'checked' ) == 'checked' ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails when upcasting checkboxes with binary attributes input type="checkbox" checked
which are parsed as checked = ""
. So this would need to check against an empty string as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't data.viewItem.checked
be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be solved in the follow-up ticket https://github.com/ckeditor/ckeditor5-list/issues/138.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pomek Yes that should work
Great thanks! Also, awesome work on the tasklist 🍻
@oskarwrobel Hey, I have a question regarding creating an interactive checklist for a GitHub's pull request template. Do you have any idea how this can be done? |
Suggested merge commit message (convention)
Feature: Introduced todo lists. Closes ckeditor/ckeditor5#1434.
TODO before merge:
increase CC to 100%https://github.com/ckeditor/ckeditor5-list/issues/126