Merge pull request #3338 from ilandikov/fix-list-item-checkboxes

fix: Toggle non-task checkbox lines in tree view

Author: claremacrae

docs/Queries/Layout.md

@@ -137,7 +137,6 @@ The `show tree` instruction enables us to see the parent/child relationships in
 
 - For now, **all child tasks and list items are displayed**, regardless of whether they match the query.
   - In the screenshot above, `Decide who to invite` did not match the `not done` query, but it is still shown.
-  - If you are using the Global Filter, any child tasks without the Global Filter are rendered without their checkbox. We are tracking this in [issue #3170](https://github.com/obsidian-tasks-group/obsidian-tasks/issues/3170).
 - Any **sorting instructions only affect the sorting of the left-most tasks** in the results list.
   - Child tasks and list items are displayed in the order that they appear in the file. They are not affected by any `sort by` instructions.
 - For now, the **tree layout is turned off by default**, whilst we explore how it should interact with the filtering instructions.

resources/sample_vaults/Tasks-Demo/Manual Testing/Smoke Testing the Tasks Plugin.md

@@ -187,6 +187,24 @@ hide postpone button
 
 ---
 
+### Toggling non-task checkboxes
+
+- Non-task checkbox are tasks without the global filter, treated as `ListItems` by the obsidian-tasks plugin
+- [ ] #task Check or uncheck non-task list items with checkbox in the query below this task
+  - [ ] I will have an `x` status  
+  - [x]  I will have a `space` status  
+  - [/] Me too with a `space` status
+
+```tasks  
+filename includes {{query.file.filename}}
+heading includes Toggling non-task checkboxes
+show tree  
+```
+
+- [ ] #task **check**: Checked all above steps for **toggling non-task list items with checkbox in a query** worked
+
+---
+
 ## Check the plugin starts OK with no `data.json` settings file
 
 - Preparation

resources/sample_vaults/Tasks-Demo/Test Data/inheritance_non_task_child.md

@@ -1,6 +1,7 @@
 -  [ ] #task task parent
     - [ ] #task task child
     - [ ] non-task child
+    - [x] non-task child status x
     - list item child
 
 ```tasks

src/Obsidian/File.ts

@@ -1,6 +1,7 @@
 import { type ListItemCache, MetadataCache, Notice, TFile, Vault, Workspace } from 'obsidian';
 import { GlobalFilter } from '../Config/GlobalFilter';
 import { type MockListItemCache, type MockTask, saveMockDataForTesting } from '../lib/MockDataCreator';
+import type { ListItem } from '../Task/ListItem';
 import type { Task } from '../Task/Task';
 import { logging } from '../lib/logging';
 import { logEndOfTaskEdit, logStartOfTaskEdit } from '../lib/LogTasksHelper';
@@ -48,8 +49,8 @@ export const replaceTaskWithTasks = async ({
     originalTask,
     newTasks,
 }: {
-    originalTask: Task;
-    newTasks: Task | Task[];
+    originalTask: ListItem;
+    newTasks: ListItem | ListItem[];
 }): Promise<void> => {
     if (vault === undefined || metadataCache === undefined || workspace === undefined) {
         errorAndNotice('Tasks: cannot use File before initializing it.');
@@ -113,8 +114,8 @@ const tryRepetitive = async ({
     workspace,
     previousTries,
 }: {
-    originalTask: Task;
-    newTasks: Task[];
+    originalTask: ListItem;
+    newTasks: ListItem[];
     vault: Vault;
     metadataCache: MetadataCache;
     workspace: Workspace;
@@ -163,7 +164,7 @@ Recommendations:
         // Finally, we can insert 1 or more lines over the original task line:
         const updatedFileLines = [
             ...fileLines.slice(0, taskLineNumber),
-            ...newTasks.map((task: Task) => task.toFileLineString()),
+            ...newTasks.map((task: ListItem) => task.toFileLineString()),
             ...fileLines.slice(taskLineNumber + 1), // Only supports single-line tasks.
         ];
 
@@ -188,7 +189,7 @@ Recommendations:
  * It may throw a WarningWorthRetrying exception in several cases that justify a retry (should be handled by the caller)
  * or an Error exception in case of an unrecoverable error.
  */
-async function getTaskAndFileLines(task: Task, vault: Vault): Promise<[number, TFile, string[]]> {
+async function getTaskAndFileLines(task: ListItem, vault: Vault): Promise<[number, TFile, string[]]> {
     if (metadataCache === undefined) throw new WarningWorthRetrying();
     // Validate our inputs.
     // For permanent failures, return nothing.

src/Renderer/QueryResultsRenderer.ts

@@ -6,6 +6,7 @@ import type { IQuery } from '../IQuery';
 import { QueryLayout } from '../Layout/QueryLayout';
 import { TaskLayout } from '../Layout/TaskLayout';
 import { PerformanceTracker } from '../lib/PerformanceTracker';
+import { replaceTaskWithTasks } from '../Obsidian/File';
 import { explainResults, getQueryForQueryRenderer } from '../Query/QueryRendererHelper';
 import { State } from '../Obsidian/Cache';
 import type { GroupDisplayHeading } from '../Query/Group/GroupDisplayHeading';
@@ -339,12 +340,43 @@ export class QueryResultsRenderer {
             return await this.addTask(taskList, taskLineRenderer, listItem, taskIndex, queryRendererParameters);
         }
 
-        return await this.addListItem(taskList, listItem);
+        return await this.addListItem(taskList, listItem, taskIndex);
     }
 
-    private async addListItem(taskList: HTMLUListElement, listItem: ListItem) {
+    private async addListItem(taskList: HTMLUListElement, listItem: ListItem, listItemIndex: number) {
         const li = createAndAppendElement('li', taskList);
 
+        if (listItem.statusCharacter) {
+            const checkbox = createAndAppendElement('input', li);
+            checkbox.classList.add('task-list-item-checkbox');
+            checkbox.type = 'checkbox';
+
+            checkbox.addEventListener('click', (event: MouseEvent) => {
+                event.preventDefault();
+                // It is required to stop propagation so that obsidian won't write the file with the
+                // checkbox (un)checked. Obsidian would write after us and overwrite our change.
+                event.stopPropagation();
+
+                // Should be re-rendered as enabled after update in file.
+                checkbox.disabled = true;
+
+                const checkedOrUncheckedListItem = listItem.checkOrUncheck();
+                replaceTaskWithTasks({ originalTask: listItem, newTasks: checkedOrUncheckedListItem });
+            });
+
+            if (listItem.statusCharacter !== ' ') {
+                checkbox.checked = true;
+                li.classList.add('is-checked');
+            }
+
+            li.classList.add('task-list-item');
+
+            // Set these to be compatible with stock obsidian lists:
+            li.setAttribute('data-task', listItem.statusCharacter.trim());
+            // Trim to ensure empty attribute for space. Same way as obsidian.
+            li.setAttribute('data-line', listItemIndex.toString());
+        }
+
         const span = createAndAppendElement('span', li);
         await this.textRenderer(
             listItem.description,
@@ -353,6 +385,15 @@ export class QueryResultsRenderer {
             this.obsidianComponent,
         );
 
+        // Unwrap the p-tag that was created by the MarkdownRenderer:
+        const pElement = span.querySelector('p');
+        if (pElement !== null) {
+            while (pElement.firstChild) {
+                span.insertBefore(pElement.firstChild, pElement);
+            }
+            pElement.remove();
+        }
+
         return li;
     }
 

src/lib/LogTasksHelper.ts

@@ -1,4 +1,4 @@
-import type { Task } from '../Task/Task';
+import type { ListItem } from '../Task/ListItem';
 import type { Logger } from './logging';
 
 /**
@@ -7,7 +7,7 @@ import type { Logger } from './logging';
  * @param codeLocation - a string description, such as 'callingFunctionName()'.
  * @param originalTask
  */
-export function logStartOfTaskEdit(logger: Logger, codeLocation: string, originalTask: Task) {
+export function logStartOfTaskEdit(logger: Logger, codeLocation: string, originalTask: ListItem) {
     logger.debug(
         `${codeLocation}: task line number: ${originalTask.taskLocation.lineNumber}. file path: "${originalTask.path}"`,
     );
@@ -20,8 +20,8 @@ export function logStartOfTaskEdit(logger: Logger, codeLocation: string, origina
  * @param codeLocation - a string description, such as 'callingFunctionName()'.
  * @param newTasks
  */
-export function logEndOfTaskEdit(logger: Logger, codeLocation: string, newTasks: Task[]) {
-    newTasks.map((task: Task, index: number) => {
+export function logEndOfTaskEdit(logger: Logger, codeLocation: string, newTasks: ListItem[]) {
+    newTasks.map((task: ListItem, index: number) => {
         // Alignment of task lines is intentionally consistent between logStartOfTaskEdit() and this:
         logger.debug(`${codeLocation} ==> ${index + 1}   : ${task.toFileLineString()}`);
     });

src/lib/MockDataCreator.ts

@@ -1,4 +1,5 @@
 import type { ListItemCache, Pos } from 'obsidian';
+import type { ListItem } from '../Task/ListItem';
 import type { Task } from '../Task/Task';
 
 // See File.test.ts for how to use this.
@@ -44,7 +45,7 @@ export type MockTogglingDataForTesting = {
  * @see saveMockDataForTesting
  */
 export function getMockDataForTesting(
-    originalTask: Task,
+    originalTask: ListItem,
     fileLines: string[],
     listItemsCache: ListItemCache[],
 ): MockTogglingDataForTesting {
@@ -86,7 +87,7 @@ export function getMockDataForTesting(
  * @param fileLines
  * @param listItemsCache
  */
-export function saveMockDataForTesting(originalTask: Task, fileLines: string[], listItemsCache: ListItemCache[]) {
+export function saveMockDataForTesting(originalTask: ListItem, fileLines: string[], listItemsCache: ListItemCache[]) {
     const everything = getMockDataForTesting(originalTask, fileLines, listItemsCache);
     console.error(`Inconsistent lines: SAVE THE OUTPUT
 data:

tests/Obsidian/Cache.test.ts

@@ -495,6 +495,7 @@ describe('cache', () => {
             "-  [ ] #task task parent
                 - [ ] #task task child
                 - [ ] non-task child
+                - [x] non-task child status x
                 - list item child
 
             \`\`\`tasks
@@ -510,6 +511,7 @@ describe('cache', () => {
             "-  [ ] #task task parent : Task
                 - [ ] #task task child : Task
                 - [ ] non-task child : ListItem
+                - [x] non-task child status x : ListItem
                 - list item child : ListItem
             "
         `);

tests/Obsidian/__test_data__/inheritance_non_task_child.json

@@ -53,15 +53,31 @@
         "parent": 0,
         "position": {
           "end": {
-            "col": 21,
+            "col": 33,
             "line": 3,
-            "offset": 98
+            "offset": 110
           },
           "start": {
             "col": 3,
             "line": 3,
             "offset": 80
           }
+        },
+        "task": "x"
+      },
+      {
+        "parent": 0,
+        "position": {
+          "end": {
+            "col": 21,
+            "line": 4,
+            "offset": 132
+          },
+          "start": {
+            "col": 3,
+            "line": 4,
+            "offset": 114
+          }
         }
       }
     ],
@@ -70,8 +86,8 @@
         "position": {
           "end": {
             "col": 21,
-            "line": 3,
-            "offset": 98
+            "line": 4,
+            "offset": 132
           },
           "start": {
             "col": 0,
@@ -85,13 +101,13 @@
         "position": {
           "end": {
             "col": 3,
-            "line": 8,
-            "offset": 164
+            "line": 9,
+            "offset": 198
           },
           "start": {
             "col": 0,
-            "line": 5,
-            "offset": 100
+            "line": 6,
+            "offset": 134
           }
         },
         "type": "code"
@@ -130,7 +146,7 @@
       }
     ]
   },
-  "fileContents": "-  [ ] #task task parent\n    - [ ] #task task child\n    - [ ] non-task child\n    - list item child\n\n```tasks\nfilename includes {{query.file.filename}}\nshow tree\n```\n",
+  "fileContents": "-  [ ] #task task parent\n    - [ ] #task task child\n    - [ ] non-task child\n    - [x] non-task child status x\n    - list item child\n\n```tasks\nfilename includes {{query.file.filename}}\nshow tree\n```\n",
   "filePath": "Test Data/inheritance_non_task_child.md",
   "getAllTags": [
     "#task",

tests/Renderer/QueryResultsRenderer.test.QueryResultsRenderer_tests_should_render_non_task_check_box_when_global_filter_is_enabled.approved.html

@@ -45,7 +45,14 @@
             <a class="tasks-edit" title="Edit task" href="#"></a>
           </span>
         </li>
-        <li><span>non-task child</span></li>
+        <li class="task-list-item" data-task="" data-line="1">
+          <input class="task-list-item-checkbox" type="checkbox" />
+          <span>non-task child</span>
+        </li>
+        <li class="is-checked task-list-item" data-task="x" data-line="2">
+          <input class="task-list-item-checkbox" type="checkbox" />
+          <span>non-task child status x</span>
+        </li>
         <li><span>list item child</span></li>
       </ul>
     </li>