Custom task states: improvements

[adoriandoran]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive suite of unit tests for the CKEditor 5 multi-state todo list plugin, covering commands, HTML conversion, keystrokes, post-fixers, and schema definitions, while configuring Vitest and Happy DOM for the test environment. The review feedback correctly identifies a DOM element leak in two test files where a new domElement is created and appended without first removing the previous instance from the document body.

[gemini-code-assist]

The previous domElement is not removed from the document body before a new one is created and appended. Since afterEach only removes the element currently referenced by domElement, the first div element will leak in the DOM during test execution. Removing the old element before reassigning domElement prevents this leak.

Suggested change

	await editor.destroy();
	domElement = document.createElement("div");
	await editor.destroy();
	domElement.remove();
	domElement = document.createElement("div");

[gemini-code-assist]

The previous domElement is not removed from the document body before a new one is created and appended. Since afterEach only removes the element currently referenced by domElement, the first div element will leak in the DOM during test execution. Removing the old element before reassigning domElement prevents this leak.

Suggested change

	await editor.destroy();
	domElement = document.createElement("div");
	await editor.destroy();
	domElement.remove();
	domElement = document.createElement("div");

[greptile-apps]

Greptile Summary

This PR adds a Vitest unit-test suite for the TodoListMultistateEditing CKEditor plugin, covering the setTaskState command, upcast/downcast conversion, post-fixer logic, keyboard cycling, and schema registration. It also wires up the necessary vitest.config.ts, tsconfig.test.json, and devDependencies.

• Five new test files exercise command refresh/execute, HTML round-tripping of data-trilium-task-state, post-fixer sync between taskState and todoListChecked, Ctrl+Shift+Enter cycle behaviour (including hidden-state skipping), and schema registration.
• tsconfig.test.json is introduced but not referenced from the root tsconfig.json, so pnpm typecheck will not cover the new test files.
• Two inline editor-recreation tests replace domElement without first removing the old host element, leaving orphan nodes in the happy-dom body across test runs.

Confidence Score: 4/5

Safe to merge — changes are test-only and do not touch production code paths.

The test infrastructure and logic are sound overall, but two tests leave orphaned DOM elements when they recreate the editor inline, and tsconfig.test.json is not wired into the project reference graph so type errors in tests escape pnpm typecheck. Neither issue affects production behaviour.

tsconfig.json (missing reference to tsconfig.test.json), todo-list-multistate-command.ts and todo-list-multistate-keystroke.ts (DOM element cleanup in inline editor-recreation tests).

Important Files Changed

Filename	Overview
packages/ckeditor5/package.json	Adds devDependencies (happy-dom, vite-plugin-svgo, vitest) and a test script to enable unit testing for the package.
packages/ckeditor5/vitest.config.ts	New Vitest configuration using happy-dom environment; discovers tests under both a top-level tests/ and src/**/tests/ patterns.
packages/ckeditor5/tsconfig.test.json	New TypeScript config for tests extending tsconfig.lib.json; not referenced from tsconfig.json so test files are excluded from project-wide type checking.
packages/ckeditor5/src/plugins/todo_list_multistate/tests/todo-list-multistate-command.ts	Tests for SetTaskStateCommand (refresh/value/execute); one test leaks the original beforeEach DOM element when it recreates the editor inline.
packages/ckeditor5/src/plugins/todo_list_multistate/tests/todo-list-multistate-keystroke.ts	Tests for Ctrl+Shift+Enter cycle; one test leaks the beforeEach DOM element when recreating the editor with a custom taskStates config.
packages/ckeditor5/src/plugins/todo_list_multistate/tests/todo-list-multistate-conversion.ts	Tests for upcast/downcast conversion of data-trilium-task-state; covers round-trip, unknown states, and editing-only CSS class isolation.
packages/ckeditor5/src/plugins/todo_list_multistate/tests/todo-list-multistate-post-fixer.ts	Tests for the post-fixer that keeps todoListChecked in sync with taskState; covers native toggle, direct attribute writes, and command flow.
packages/ckeditor5/src/plugins/todo_list_multistate/tests/todo-list-multistate-schema.ts	Tests that the plugin registers the setTaskState command and extends the model schema to allow taskState on $block nodes.
pnpm-lock.yaml	Adds new dev dependencies for @triliumnext/ckeditor5; a pre-existing integrity hash for @electron/node-gyp (git-hosted) was dropped during regeneration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[vitest run] --> B{include patterns}
    B --> C["src/**/tests/**/*.[jt]s"]
    B --> D["tests/**/*.[jt]s"]
    C --> E[todo-list-multistate-*.ts]

    E --> F[ClassicEditor.create]
    F --> G[TodoListMultistateEditing plugin]
    G --> H[setTaskState command]
    G --> I[upcast: data-trilium-task-state to taskState]
    G --> J[downcast: taskState to data-trilium-task-state]
    G --> K[post-fixer: sync taskState and todoListChecked]
    G --> L[Ctrl+Shift+Enter keystroke cycle]

    H --> M{state?}
    M -- none --> N[remove taskState, set checked=false]
    M -- done --> O[remove taskState, set checked=true]
    M -- custom --> P[set taskState, post-fixer drives checked]

    K --> Q{checkedChanged only?}
    Q -- yes --> R[remove taskState]
    Q -- no / stateChanged --> S[sync todoListChecked to isCompleted]

Loading

_{Reviews (1): Last reviewed commit: "custom task states: add tests" | Re-trigger Greptile}

[greptile-apps]

Leaked DOM element when recreating the editor

editor.destroy() cleans up the CKEditor instance but the original domElement (created in beforeEach) remains attached to document.body once domElement is reassigned to the new element. domElement.remove() before the reassignment ensures the old host element is detached. The same pattern appears in todo-list-multistate-keystroke.ts.

Suggested change

	it("custom isCompleted=true state ticks the native checkbox via the post-fixer", async () => {
	await editor.destroy();
	domElement = document.createElement("div");
	it("custom isCompleted=true state ticks the native checkbox via the post-fixer", async () => {
	await editor.destroy();
	domElement.remove();
	domElement = document.createElement("div");

[greptile-apps]

Leaked DOM element when recreating the editor

Same pattern as in todo-list-multistate-command.ts: the original domElement from beforeEach remains in document.body after domElement is reassigned. Add domElement.remove() before reassigning to avoid accumulating orphaned elements across test runs.

Suggested change

	it("skips hidden states in the cycle", async () => {
	await editor.destroy();
	domElement = document.createElement("div");
	document.body.appendChild(domElement);
	it("skips hidden states in the cycle", async () => {
	await editor.destroy();
	domElement.remove();
	domElement = document.createElement("div");
	document.body.appendChild(domElement);

[greptile-apps]

domElement.remove() called before editor.destroy() in every afterEach

All five test files remove the host element from the DOM before calling editor.destroy(). CKEditor's teardown may still need the element to be connected (e.g. to clean up MutationObserver, focus traps, or view observers). The recommended CKEditor pattern is await editor.destroy() first, then detach the element. Because destroy() is returned (not awaited), a rejection from destroy() would also be silently dropped in the happy-dom environment. This same ordering issue exists in all five test files.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!