Desktop, Mobile: Fixes #11845: Fixes issues with converting nested lists between bullet and numbered.

[rabbabansh]

Fixes #11845 now, this"

• test
  • test
  • test
• test

converts to:

Checklists also convert nested lists fully, rather than just converting the first indent to checklists.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[rabbabansh]

I have read the CLA Document and I hereby sign the CLA.

[personalizedrefrigerator]

Thank you for working on this! Be sure to also update the automated tests in packages/editor/ with the expected output.

[rabbabansh]

Done! Please review

[personalizedrefrigerator]

Thank you! Be sure to update the existing tests, too — one is failing:

➤ YN0000: [@joplin/editor]:   FAIL CodeMirror/markdown/markdownCommands.toggleList.test.ts
➤ YN0000: [@joplin/editor]:   ● markdownCommands.toggleList › should toggle a numbered list without changing its sublists
➤ YN0000: [@joplin/editor]: 
➤ YN0000: [@joplin/editor]:     expect(received).toBe(expected) // Object.is equality
➤ YN0000: [@joplin/editor]: 
➤ YN0000: [@joplin/editor]:     - Expected  - 3
➤ YN0000: [@joplin/editor]:     + Received  + 3
➤ YN0000: [@joplin/editor]: 
➤ YN0000: [@joplin/editor]:       - [ ] Foo
➤ YN0000: [@joplin/editor]:       - [ ] Bar
➤ YN0000: [@joplin/editor]:       - [ ] Baz
➤ YN0000: [@joplin/editor]:     - 	- Test
➤ YN0000: [@joplin/editor]:     + 	- [ ] Test
➤ YN0000: [@joplin/editor]:     - 	- of
➤ YN0000: [@joplin/editor]:     + 	- [ ] of
➤ YN0000: [@joplin/editor]:     - 	- sublists
➤ YN0000: [@joplin/editor]:     + 	- [ ] sublists
➤ YN0000: [@joplin/editor]:       - [ ] Foo
➤ YN0000: [@joplin/editor]: 
➤ YN0000: [@joplin/editor]:       191 |
➤ YN0000: [@joplin/editor]:       192 | 		toggleList(ListType.CheckList)(editor);
➤ YN0000: [@joplin/editor]:     > 193 | 		expect(editor.state.doc.toString()).toBe(
➤ YN0000: [@joplin/editor]:           | 		                                    ^
➤ YN0000: [@joplin/editor]:       194 | 			'- [ ] Foo\n- [ ] Bar\n- [ ] Baz\n\t- Test\n\t- of\n\t- sublists\n- [ ] Foo',
➤ YN0000: [@joplin/editor]:       195 | 		);
➤ YN0000: [@joplin/editor]:       196 | 	});
➤ YN0000: [@joplin/editor]: 
➤ YN0000: [@joplin/editor]:       at Object.<anonymous> (CodeMirror/markdown/markdownCommands.toggleList.test.ts:193:39)

[rabbabansh]

That tests passes now! I rewrote the expected results for a nested checklist that gives checklists to the sublists as well.

[laurent22]

@personalizedrefrigerator, please let me know when you think it's ready to merge, as I don't know enough about the CM6 implementation to review

[rabbabansh]

any updates @personalizedrefrigerator ?

[personalizedrefrigerator]

any updates @personalizedrefrigerator ?

The code looks good to me! I plan to test the change locally before approving the pull request.

[personalizedrefrigerator]

I've tested this locally. I've noticed that changing the list type with the cursor anywhere in the toplevel list (with an empty selection) changes the list types of all sublists. The description of #11845 includes only the case where all text is selected.

• Note: This is different from how Joplin's Rich Text Editor behaves and thus may not be desired behavior (but to be confirmed by @laurent22). For comparison, moving the cursor to a bulleted list in the Rich Text Editor with bulleted sublists, then clicking "numbered list" only changes the outer list.
• For example, with this pull request clicking "numbered list" with the following

  - This is a test. Cursor here ->> |
     - Sublist
        - Sub sublist.
  - Testing
  - Test.
  

  produces

  1. This is a test. Cursor here ->> |
     1. Sublist
        1. Sub sublist.
  2. Testing
  3. Test.
  

  Whereas in the Rich Text Editor, it would preserve the bulleted sublists.

Thank you @rabbabansh for the work you've done on this so far!

[laurent22]

Not sure what's best here? What do other editors do in this situation?

[laurent22]

Note: This is different from how Joplin's Rich Text Editor behaves and thus may not be desired behavior (but to be confirmed by @laurent22). For comparison, moving the cursor to a bulleted list in the Rich Text Editor with bulleted sublists, then clicking "numbered list" only changes the outer list.

I agree that's not ideal. I think as a general rule we should limit all kind of "smart" behaviour when it comes to lists so in this case it should:

• Either changes only what's selected
• Or if nothing is selected it should just change the current line

Anything else may or may not be what the user wants and we should avoid that. I'd rather have the editor be more plain but with a predictable behaviour.

@rabbabansh, are you able to complete this pull request?

[laurent22]

Closing stale pull request