Merge pull request #3678 from ilandikov/refactor/create-HTMLLIElement

refactor: create `HTMLLIElement` outside of `TaskLineRenderer` methods

Author: claremacrae

src/Obsidian/InlineRenderer.ts

@@ -5,7 +5,7 @@ import { TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
 import { QueryLayoutOptions } from '../Layout/QueryLayoutOptions';
 import { TasksFile } from '../Scripting/TasksFile';
 import { Task } from '../Task/Task';
-import { TaskLineRenderer } from '../Renderer/TaskLineRenderer';
+import { TaskLineRenderer, createAndAppendElement } from '../Renderer/TaskLineRenderer';
 import { TaskLocation } from '../Task/TaskLocation';
 
 /**
@@ -137,8 +137,9 @@ export class InlineRenderer {
             }
             const dataLine: string = renderedElement.getAttr('data-line') ?? '0';
             const taskIndex: number = Number.parseInt(dataLine, 10);
-            const taskElement = await taskLineRenderer.renderTaskLine({
-                parentUlElement: element,
+            const taskElement = createAndAppendElement('li', element);
+            await taskLineRenderer.renderTaskLine({
+                li: taskElement,
                 task,
                 taskIndex,
                 isTaskInQueryFile: true,

src/Renderer/HtmlQueryResultsRenderer.ts

@@ -131,11 +131,9 @@ export class HtmlQueryResultsRenderer extends QueryResultsRendererBase {
     }
 
     protected async addListItem(listItem: ListItem, listItemIndex: number, children: ListItem[]): Promise<void> {
-        const listItemElement = await this.taskLineRenderer.renderListItem(
-            this.currentULElement(),
-            listItem,
-            listItemIndex,
-        );
+        const taskList = this.currentULElement();
+        const listItemElement = createAndAppendElement('li', taskList);
+        await this.taskLineRenderer.renderListItem(listItemElement, listItem, listItemIndex);
 
         if (children.length > 0) {
             // TODO re-extract the method to include this back
@@ -151,8 +149,10 @@ export class HtmlQueryResultsRenderer extends QueryResultsRendererBase {
 
     protected async addTask(task: Task, taskIndex: number, children: ListItem[]): Promise<void> {
         const isFilenameUnique = this.isFilenameUnique({ task }, this.queryRendererParameters.allMarkdownFiles());
-        const listItem = await this.taskLineRenderer.renderTaskLine({
-            parentUlElement: this.currentULElement(),
+        const parentUlElement = this.currentULElement();
+        const listItem = createAndAppendElement('li', parentUlElement);
+        await this.taskLineRenderer.renderTaskLine({
+            li: listItem,
             task,
             taskIndex,
             isTaskInQueryFile: this.filePath === task.path,

src/Renderer/TaskLineRenderer.ts

@@ -2,19 +2,19 @@ import type { Moment } from 'moment';
 import { type App, Component, MarkdownRenderer } from 'obsidian';
 import { GlobalFilter } from '../Config/GlobalFilter';
 import { TASK_FORMATS, getSettings } from '../Config/Settings';
+import type { AllTaskDateFields } from '../DateTime/DateFieldTypes';
+import { splitDateText } from '../DateTime/Postponer';
 import type { QueryLayoutOptions } from '../Layout/QueryLayoutOptions';
 import { TaskLayoutComponent, type TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
 import { replaceTaskWithTasks } from '../Obsidian/File';
 import { StatusRegistry } from '../Statuses/StatusRegistry';
+import type { ListItem } from '../Task/ListItem';
 import { Task } from '../Task/Task';
 import { TaskRegularExpressions } from '../Task/TaskRegularExpressions';
+import { DateMenu } from '../ui/Menus/DateMenu';
+import { promptForDate } from '../ui/Menus/DatePicker';
 import { StatusMenu } from '../ui/Menus/StatusMenu';
-import type { AllTaskDateFields } from '../DateTime/DateFieldTypes';
 import { defaultTaskSaver, showMenu } from '../ui/Menus/TaskEditingMenu';
-import { promptForDate } from '../ui/Menus/DatePicker';
-import { splitDateText } from '../DateTime/Postponer';
-import { DateMenu } from '../ui/Menus/DateMenu';
-import type { ListItem } from '../Task/ListItem';
 import { TaskFieldRenderer } from './TaskFieldRenderer';
 
 /**
@@ -126,7 +126,7 @@ export class TaskLineRenderer {
      *
      * @returns an HTML rendered List Item element (LI) for a task.
      * @note Output is based on the {@link DefaultTaskSerializer}'s format, with default (emoji) symbols
-     * @param parentUlElement HTML element where the task shall be rendered.
+     * @param li HTML element for the rendered task.
      * @param task The task to be rendered.
      * @param taskIndex Task's index in the list. This affects `data-line` data attributes of the list item.
      * @param isTaskInQueryFile
@@ -135,19 +135,18 @@ export class TaskLineRenderer {
      *                         the file name only. If set to `true`, the full path will be returned.
      */
     public async renderTaskLine({
-        parentUlElement,
+        li,
         task,
         taskIndex,
         isTaskInQueryFile,
         isFilenameUnique,
     }: {
-        parentUlElement: HTMLElement;
+        li: HTMLLIElement;
         task: Task;
         taskIndex: number;
         isTaskInQueryFile: boolean;
         isFilenameUnique?: boolean;
-    }): Promise<HTMLLIElement> {
-        const li = createAndAppendElement('li', parentUlElement);
+    }): Promise<void> {
         li.classList.add('task-list-item', 'plugin-tasks-list-item');
 
         const textSpan = createAndAppendElement('span', li);
@@ -202,8 +201,6 @@ export class TaskLineRenderer {
         if (this.queryLayoutOptions.shortMode) {
             this.addTooltip(task, textSpan, isFilenameUnique);
         }
-
-        return li;
     }
 
     private async taskToHtml(
@@ -459,9 +456,7 @@ export class TaskLineRenderer {
         });
     }
 
-    public async renderListItem(taskList: HTMLUListElement, listItem: ListItem, listItemIndex: number) {
-        const li = createAndAppendElement('li', taskList);
-
+    public async renderListItem(li: HTMLLIElement, listItem: ListItem, listItemIndex: number): Promise<HTMLLIElement> {
         if (listItem.statusCharacter) {
             const checkbox = createAndAppendElement('input', li);
             checkbox.classList.add('task-list-item-checkbox');

tests/Renderer/TaskLineRenderer.test.ts

@@ -10,7 +10,7 @@ import { QueryLayoutOptions } from '../../src/Layout/QueryLayoutOptions';
 import { TaskLayoutComponent, TaskLayoutOptions, taskLayoutComponents } from '../../src/Layout/TaskLayoutOptions';
 import { DateParser } from '../../src/DateTime/DateParser';
 import type { TextRenderer } from '../../src/Renderer/TaskLineRenderer';
-import { TaskLineRenderer } from '../../src/Renderer/TaskLineRenderer';
+import { TaskLineRenderer, createAndAppendElement } from '../../src/Renderer/TaskLineRenderer';
 import type { Task } from '../../src/Task/Task';
 import { TaskRegularExpressions } from '../../src/Task/TaskRegularExpressions';
 import { verifyWithFileExtension } from '../TestingTools/ApprovalTestHelpers';
@@ -47,12 +47,16 @@ async function renderListItem(
         taskLayoutOptions: taskLayoutOptions ?? new TaskLayoutOptions(),
         queryLayoutOptions: queryLayoutOptions ?? new QueryLayoutOptions(),
     });
-    return await taskLineRenderer.renderTaskLine({
-        parentUlElement: document.createElement('div'),
+    const divElement = document.createElement('div');
+    const li = createAndAppendElement('li', divElement);
+    await taskLineRenderer.renderTaskLine({
+        li: li,
         task: task,
         taskIndex: 0,
         isTaskInQueryFile: true,
     });
+
+    return li;
 }
 
 function getTextSpan(listItem: HTMLElement) {
@@ -86,32 +90,6 @@ afterEach(() => {
 });
 
 describe('task line rendering - HTML', () => {
-    it('should render only one List Item for the UL and return it with renderTaskLine()', async () => {
-        const ulElement = document.createElement('ul');
-        const taskLineRenderer = new TaskLineRenderer({
-            textRenderer: mockTextRenderer,
-            obsidianApp: mockApp,
-            obsidianComponent: null,
-            taskLayoutOptions: new TaskLayoutOptions(),
-            queryLayoutOptions: new QueryLayoutOptions(),
-        });
-        const listItem = await taskLineRenderer.renderTaskLine({
-            parentUlElement: ulElement,
-            task: new TaskBuilder().build(),
-            taskIndex: 0,
-            isTaskInQueryFile: true,
-        });
-
-        // Just one element
-        expect(ulElement.children.length).toEqual(1);
-
-        // It is the rendered one
-        expect(ulElement.children[0]).toEqual(listItem);
-
-        // And it is a ListItem
-        expect(listItem.nodeName).toEqual('LI');
-    });
-
     it('creates the correct span structure for a basic task inside a List Item', async () => {
         const taskLine = '- [ ] This is a simple task';
         const task = fromLine({