Skip to content

Conversation

@MrHeer
Copy link
Contributor

@MrHeer MrHeer commented Nov 22, 2019

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
New tests added? not needed
Fixed tickets #1724
License MIT

Description

Auto update others checked state while change a task

@MrHeer
Copy link
Contributor Author

MrHeer commented Nov 22, 2019

I need to take a moment to think about this.updateParentsCheckBoxState(checkbox)

@MrHeer
Copy link
Contributor Author

MrHeer commented Nov 24, 2019

Can you give me some test case? @Jocs

@Jocs
Copy link
Member

Jocs commented Nov 24, 2019

@MrHeer I just think these two cases:

- [x] foo
  
  - [ ] bar
    
    - [ ] zar

---

- [x] - [x] - [x] foo
  
  - [ ] bar
  
  - [ ] zar

@MrHeer
Copy link
Contributor Author

MrHeer commented Nov 24, 2019

Thank you very much.
There is no problem in the first case test.
I don’t understand the second case.
Screen record from 2019-11-24 23 50 51

@Jocs
Copy link
Member

Jocs commented Nov 24, 2019

The second will be less use, but it is a legal grammar I think.

@MrHeer
Copy link
Contributor Author

MrHeer commented Nov 24, 2019

I mean I can't understand their logic, I don't known how to update the others status after a task checked

@Jocs
Copy link
Member

Jocs commented Nov 26, 2019

Distinguishing parent-child relationships by DOM structure?

@MrHeer
Copy link
Contributor Author

MrHeer commented Nov 27, 2019

Thank you, I'll try to sovle

@MrHeer
Copy link
Contributor Author

MrHeer commented Nov 29, 2019

@Jocs If distinguishing parent-child relationships by DOM structure, the logic is right.

@renehummen
Copy link

I would love to have this feature in Mark Text.

@MrHeer Is there an issue that is blocking this PR from your side?

If it is because of the failing CI build, it seems to me that it is failing due to an PR-independent build error related to vscode-ripgrep.

@MrHeer
Copy link
Contributor Author

MrHeer commented Jan 12, 2020

@renehummen The author think my code had some problems, but I'm not clear that.

@renehummen
Copy link

@Jocs If distinguishing parent-child relationships by DOM structure, the logic is right.

@MrHeer Can this PR also handle the following cases?

- [ ] task 1
  - [ ] subtask 1.1
    - [ ] subsubtask 1.1.1
    - [ ] subsubtask 1.1.2
  - [ ] subtask 1.2
    - [ ] subsubtask 1.2.1
    - [ ] subsubtask 1.2.2
- [x] task 2
  - [x] subtask 2.1
    - [x] subsubtask 2.1.1
    - [x] subsubtask 2.1.2
  - [x] subtask 2.2
    - [x] subsubtask 2.2.1
    - [x] subsubtask 2.2.2

... where toggling task completion leads to the following behavior:

  1. Toggling subtask 1.1 to complete, toggles completion of subsubtasks 1.1.1 and 1.1.2
  2. Toggling subsubtasks 1.2.1 and 1.2.2 to complete, toggles completion of subtask 1.2
  3. (1) and (2) combined also toggle completion of task 1

I am not entirely sure what the best behavior for untoggling would be. I would suggest:

  1. Untoggling completion of subtask 2.1 toggles subsubtasks 2.1.1 and 2.1.2 to incomplete
  2. Untoggling competion of subsubtasks 2.2.1 and 2.2.2 toggles subtask 2.2 to incomplete
  3. (3) and (4) combined also toggle task 2 to incomplete

@MrHeer
Copy link
Contributor Author

MrHeer commented Jan 12, 2020

This PR handle it like this:
Screen record from 2020-01-12 21 30 21

@fxha
Copy link
Contributor

fxha commented Jan 13, 2020

@MrHeer I think the best option would be to add a settings option for this feature, so the user can decide whether the feature should be enabled or all checkboxes must be checked.

@MrHeer
Copy link
Contributor Author

MrHeer commented Jan 14, 2020

@fxha Thank you, I will add a settings option for this feature when I have time.

@MrHeer MrHeer force-pushed the smarter-task-list branch from 8f53b9e to 0bda939 Compare January 18, 2020 10:26
@MrHeer
Copy link
Contributor Author

MrHeer commented Jan 18, 2020

@fxha Hi, I have updated, can you review?

@Jocs
Copy link
Member

Jocs commented Feb 6, 2020

@MrHeer Sorry for reply so late, Did you have finished this PR, so we can review it?

@MrHeer
Copy link
Contributor Author

MrHeer commented Feb 6, 2020

Yes, you can review it.

@MrHeer MrHeer requested a review from Jocs February 7, 2020 03:10
@Jocs
Copy link
Member

Jocs commented Feb 8, 2020

I have reviewed the code again, and run it locally, and found a small problem. When initializing the editor, autoCheck was not passed to the editor, causing autoCheck to use the default values in muya instead of user settings.

@MrHeer
Copy link
Contributor Author

MrHeer commented Feb 9, 2020

@Jocs Thank you very much. I have updated, please review again.

1. fix autoCheck was not passed to the editor
2. put getParentCheckBox and cumputeChecboxStatus functions in utils folder.
3. put setCheckBoxState, updateParentsCheckBoxState, updateChildrenCheckBoxState
   and listItemCheckBoxClick methods in clickCtrl.js file.
@MrHeer MrHeer force-pushed the smarter-task-list branch from f4d37d8 to 9a27279 Compare February 9, 2020 09:39
@Jocs
Copy link
Member

Jocs commented Feb 11, 2020

@MrHeer Thanks very much for your contribution!

@Jocs Jocs merged commit 3103aee into marktext:develop Feb 11, 2020
@MrHeer
Copy link
Contributor Author

MrHeer commented Feb 12, 2020

It's my pleasure.

@MrHeer MrHeer deleted the smarter-task-list branch February 12, 2020 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants