Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2610 (2024-01-23 11:17:24)

refactor: remove inheritance between `TaskLayout` and `QueryLayout`

# Description

- remove inheritance between `TaskLayout` and `QueryLayout`
- move `TaskLayoutComponent` to `TaskLayoutOptions`
- extract `LayoutHelpers`
- remove `taskListHiddenClasses()` from TaskLayout
- extract "factory functions" in `TaskFieldRenderer`

## Motivation and Context

Way towards removal of `TaskLayout` and `QueryLayout` classes which are needed to only provide a list of hidden classes.

## How has this been tested?

- unit test (modified)
- smoke test in local demo vault

## Types of changes

Internal changes:

- [x] **Refactor** (prefix: `refactor` - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Checklist

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

- [x] My contribution follow this project's [contributing guide](https://publish.obsidian.md/tasks-contributing)
- [x] I agree to follow this project's [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md)



Patch:
diff --git a/src/Layout/LayoutHelpers.ts b/src/Layout/LayoutHelpers.ts
new file mode 100644
index 00000000..b57dc054
--- /dev/null
+++ b/src/Layout/LayoutHelpers.ts
@@ -0,0 +1,9 @@
+export function generateHiddenClassForTaskList(taskListHiddenClasses: string[], hide: boolean, component: string) {
+    if (hide) {
+        taskListHiddenClasses.push(hiddenComponentClassName(component));
+    }
+}
+
+function hiddenComponentClassName(component: string) {
+    return `tasks-layout-hide-${component}`;
+}
diff --git a/src/Layout/QueryLayout.ts b/src/Layout/QueryLayout.ts
new file mode 100644
index 00000000..504d8444
--- /dev/null
+++ b/src/Layout/QueryLayout.ts
@@ -0,0 +1,36 @@
+import { generateHiddenClassForTaskList } from './LayoutHelpers';
+import { QueryLayoutOptions } from './QueryLayoutOptions';
+
+export class QueryLayout {
+    protected queryLayoutOptions: QueryLayoutOptions;
+
+    constructor(queryLayoutOptions?: QueryLayoutOptions) {
+        if (queryLayoutOptions) {
+            this.queryLayoutOptions = queryLayoutOptions;
+        } else {
+            this.queryLayoutOptions = new QueryLayoutOptions();
+        }
+    }
+
+    public applyQueryLayoutOptions() {
+        const taskListHiddenClasses: string[] = [];
+        const componentsToGenerateClassesOnly: [boolean, string][] = [
+            // The following components are handled in QueryRenderer.ts and thus are not part of the same flow that
+            // hides TaskLayoutComponent items. However, we still want to have 'tasks-layout-hide' items for them
+            // (see https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1866).
+            // This can benefit from some refactoring, i.e. render these components in a similar flow rather than
+            // separately.
+            [this.queryLayoutOptions.hideUrgency, 'urgency'],
+            [this.queryLayoutOptions.hideBacklinks, 'backlinks'],
+            [this.queryLayoutOptions.hideEditButton, 'edit-button'],
+            [this.queryLayoutOptions.hidePostponeButton, 'postpone-button'],
+        ];
+        for (const [hide, component] of componentsToGenerateClassesOnly) {
+            generateHiddenClassForTaskList(taskListHiddenClasses, hide, component);
+        }
+
+        if (this.queryLayoutOptions.shortMode) taskListHiddenClasses.push('tasks-layout-short-mode');
+
+        return taskListHiddenClasses;
+    }
+}
diff --git a/src/Layout/TaskLayout.ts b/src/Layout/TaskLayout.ts
index d33ffa87..3580e2d1 100644
--- a/src/Layout/TaskLayout.ts
+++ b/src/Layout/TaskLayout.ts
@@ -1,112 +1,34 @@
+import { generateHiddenClassForTaskList } from './LayoutHelpers';
 import { TaskLayoutOptions } from './TaskLayoutOptions';
-import { QueryLayoutOptions } from './QueryLayoutOptions';
-
-export type TaskLayoutComponent =
-    // NEW_TASK_FIELD_EDIT_REQUIRED
-    | 'description'
-    | 'priority'
-    | 'recurrenceRule'
-    | 'createdDate'
-    | 'startDate'
-    | 'scheduledDate'
-    | 'dueDate'
-    | 'doneDate'
-    | 'cancelledDate'
-    | 'blockedBy'
-    | 'id'
-    | 'blockLink';
-
-// The order here determines the order that task fields are rendered and written to markdown.
-export const taskLayoutComponents: TaskLayoutComponent[] = [
-    // NEW_TASK_FIELD_EDIT_REQUIRED
-    'description',
-    'id',
-    'blockedBy',
-    'priority',
-    'recurrenceRule',
-    'createdDate',
-    'startDate',
-    'scheduledDate',
-    'dueDate',
-    'cancelledDate',
-    'doneDate',
-    'blockLink',
-];
-
-export class QueryLayout {
-    protected queryLayoutOptions: QueryLayoutOptions;
-
-    constructor(queryLayoutOptions?: QueryLayoutOptions) {
-        if (queryLayoutOptions) {
-            this.queryLayoutOptions = queryLayoutOptions;
-        } else {
-            this.queryLayoutOptions = new QueryLayoutOptions();
-        }
-    }
-
-    protected applyQueryLayoutOptions(taskListHiddenClasses: string[]) {
-        const componentsToGenerateClassesOnly: [boolean, string][] = [
-            // The following components are handled in QueryRenderer.ts and thus are not part of the same flow that
-            // hides TaskLayoutComponent items. However, we still want to have 'tasks-layout-hide' items for them
-            // (see https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1866).
-            // This can benefit from some refactoring, i.e. render these components in a similar flow rather than
-            // separately.
-            [this.queryLayoutOptions.hideUrgency, 'urgency'],
-            [this.queryLayoutOptions.hideBacklinks, 'backlinks'],
-            [this.queryLayoutOptions.hideEditButton, 'edit-button'],
-            [this.queryLayoutOptions.hidePostponeButton, 'postpone-button'],
-        ];
-        for (const [hide, component] of componentsToGenerateClassesOnly) {
-            generateHiddenClassForTaskList(taskListHiddenClasses, hide, component);
-        }
-
-        if (this.queryLayoutOptions.shortMode) taskListHiddenClasses.push('tasks-layout-short-mode');
-    }
-}
-
-function generateHiddenClassForTaskList(taskListHiddenClasses: string[], hide: boolean, component: string) {
-    if (hide) {
-        taskListHiddenClasses.push(hiddenComponentClassName(component));
-    }
-}
-
-function hiddenComponentClassName(component: string) {
-    return `tasks-layout-hide-${component}`;
-}
 
 /**
  * This represents the desired layout of tasks when they are rendered in a given configuration.
  * The layout is used when flattening the task to a string and when rendering queries, and can be
  * modified by applying {@link TaskLayoutOptions} objects.
  */
-export class TaskLayout extends QueryLayout {
-    public taskListHiddenClasses(): string[] {
-        return this._taskListHiddenClasses;
-    }
+export class TaskLayout {
     private taskLayoutOptions: TaskLayoutOptions;
-    private _taskListHiddenClasses: string[] = [];
-
-    constructor(taskLayoutOptions?: TaskLayoutOptions, queryLayoutOptions?: QueryLayoutOptions) {
-        super(queryLayoutOptions);
 
+    constructor(taskLayoutOptions?: TaskLayoutOptions) {
         if (taskLayoutOptions) {
             this.taskLayoutOptions = taskLayoutOptions;
         } else {
             this.taskLayoutOptions = new TaskLayoutOptions();
         }
-        this.applyTaskLayoutOptions();
-        this.applyQueryLayoutOptions(this._taskListHiddenClasses);
     }
-    private applyTaskLayoutOptions() {
+    public applyTaskLayoutOptions() {
+        const taskListHiddenClasses: string[] = [];
         this.taskLayoutOptions.toggleableComponents.forEach((component) => {
             generateHiddenClassForTaskList(
-                this._taskListHiddenClasses,
+                taskListHiddenClasses,
                 !this.taskLayoutOptions.isShown(component),
                 component,
             );
         });
 
         // Tags are hidden, rather than removed. See tasks-layout-hide-tags in styles.css.
-        generateHiddenClassForTaskList(this._taskListHiddenClasses, !this.taskLayoutOptions.areTagsShown(), 'tags');
+        generateHiddenClassForTaskList(taskListHiddenClasses, !this.taskLayoutOptions.areTagsShown(), 'tags');
+
+        return taskListHiddenClasses;
     }
 }
diff --git a/src/Layout/TaskLayoutOptions.ts b/src/Layout/TaskLayoutOptions.ts
index f773aab5..5cea27ed 100644
--- a/src/Layout/TaskLayoutOptions.ts
+++ b/src/Layout/TaskLayoutOptions.ts
@@ -1,4 +1,34 @@
-import { type TaskLayoutComponent, taskLayoutComponents } from './TaskLayout';
+export type TaskLayoutComponent =
+    // NEW_TASK_FIELD_EDIT_REQUIRED
+    | 'description'
+    | 'priority'
+    | 'recurrenceRule'
+    | 'createdDate'
+    | 'startDate'
+    | 'scheduledDate'
+    | 'dueDate'
+    | 'doneDate'
+    | 'cancelledDate'
+    | 'blockedBy'
+    | 'id'
+    | 'blockLink';
+
+// The order here determines the order that task fields are rendered and written to markdown.
+export const taskLayoutComponents: TaskLayoutComponent[] = [
+    // NEW_TASK_FIELD_EDIT_REQUIRED
+    'description',
+    'id',
+    'blockedBy',
+    'priority',
+    'recurrenceRule',
+    'createdDate',
+    'startDate',
+    'scheduledDate',
+    'dueDate',
+    'cancelledDate',
+    'doneDate',
+    'blockLink',
+];
 
 /**
  * Various rendering options of tasks in a query.
diff --git a/src/Renderer/QueryRenderer.ts b/src/Renderer/QueryRenderer.ts
index 75debd04..ec8a9c9a 100644
--- a/src/Renderer/QueryRenderer.ts
+++ b/src/Renderer/QueryRenderer.ts
@@ -2,6 +2,7 @@ import type { EventRef, MarkdownPostProcessorContext } from 'obsidian';
 import { App, Keymap, MarkdownRenderChild, MarkdownRenderer, TFile } from 'obsidian';
 import { GlobalFilter } from '../Config/GlobalFilter';
 import { GlobalQuery } from '../Config/GlobalQuery';
+import { QueryLayout } from '../Layout/QueryLayout';
 import { DateFallback } from '../Task/DateFallback';
 
 import type { IQuery } from '../IQuery';
@@ -221,10 +222,11 @@ class QueryRenderChild extends MarkdownRenderChild {
     }
 
     private async createTaskList(tasks: Task[], content: HTMLDivElement): Promise<void> {
-        const layout = new TaskLayout(this.query.taskLayoutOptions, this.query.queryLayoutOptions);
+        const layout = new TaskLayout(this.query.taskLayoutOptions);
+        const queryLayout = new QueryLayout(this.query.queryLayoutOptions);
         const taskList = content.createEl('ul');
         taskList.addClasses(['contains-task-list', 'plugin-tasks-query-result']);
-        taskList.addClasses(layout.taskListHiddenClasses());
+        taskList.addClasses([...layout.applyTaskLayoutOptions(), ...queryLayout.applyQueryLayoutOptions()]);
         const groupingAttribute = this.getGroupingAttribute();
         if (groupingAttribute && groupingAttribute.length > 0) taskList.dataset.taskGroupBy = groupingAttribute;
 
diff --git a/src/Renderer/TaskFieldRenderer.ts b/src/Renderer/TaskFieldRenderer.ts
index 77222f46..425759ad 100644
--- a/src/Renderer/TaskFieldRenderer.ts
+++ b/src/Renderer/TaskFieldRenderer.ts
@@ -1,7 +1,8 @@
 import type { Moment } from 'moment';
+
+import type { TaskLayoutComponent } from '../Layout/TaskLayoutOptions';
 import { PriorityTools } from '../lib/PriorityTools';
 import type { Task } from '../Task/Task';
-import type { TaskLayoutComponent } from '../Layout/TaskLayout';
 
 export class TaskFieldRenderer {
     private readonly data = taskFieldHTMLData;
@@ -139,43 +140,34 @@ export class TaskFieldHTMLData {
     }
 }
 
-const taskFieldHTMLData: { [c in TaskLayoutComponent]: TaskFieldHTMLData } = {
-    // NEW_TASK_FIELD_EDIT_REQUIRED
-    createdDate: new TaskFieldHTMLData('task-created', 'taskCreated', TaskFieldHTMLData.dateAttributeCalculator),
-    dueDate: new TaskFieldHTMLData('task-due', 'taskDue', TaskFieldHTMLData.dateAttributeCalculator),
-    startDate: new TaskFieldHTMLData('task-start', 'taskStart', TaskFieldHTMLData.dateAttributeCalculator),
-    scheduledDate: new TaskFieldHTMLData('task-scheduled', 'taskScheduled', TaskFieldHTMLData.dateAttributeCalculator),
-    doneDate: new TaskFieldHTMLData('task-done', 'taskDone', TaskFieldHTMLData.dateAttributeCalculator),
-    cancelledDate: new TaskFieldHTMLData('task-cancelled', 'taskCancelled', TaskFieldHTMLData.dateAttributeCalculator),
-
-    description: new TaskFieldHTMLData(
-        'task-description',
-        TaskFieldHTMLData.noAttributeName,
-        TaskFieldHTMLData.noAttributeValueCalculator,
-    ),
-    recurrenceRule: new TaskFieldHTMLData(
-        'task-recurring',
+function createFieldWithoutDataAttributes(className: string) {
+    return new TaskFieldHTMLData(
+        className,
         TaskFieldHTMLData.noAttributeName,
         TaskFieldHTMLData.noAttributeValueCalculator,
-    ),
+    );
+}
+
+function createDateField(className: string, attributeName: string) {
+    return new TaskFieldHTMLData(className, attributeName, TaskFieldHTMLData.dateAttributeCalculator);
+}
+
+const taskFieldHTMLData: { [c in TaskLayoutComponent]: TaskFieldHTMLData } = {
+    // NEW_TASK_FIELD_EDIT_REQUIRED
+    createdDate: createDateField('task-created', 'taskCreated'),
+    dueDate: createDateField('task-due', 'taskDue'),
+    startDate: createDateField('task-start', 'taskStart'),
+    scheduledDate: createDateField('task-scheduled', 'taskScheduled'),
+    doneDate: createDateField('task-done', 'taskDone'),
+    cancelledDate: createDateField('task-cancelled', 'taskCancelled'),
 
     priority: new TaskFieldHTMLData('task-priority', 'taskPriority', (_component, task) => {
         return PriorityTools.priorityNameUsingNormal(task.priority).toLocaleLowerCase();
     }),
 
-    blockedBy: new TaskFieldHTMLData(
-        'task-blockedBy',
-        TaskFieldHTMLData.noAttributeName,
-        TaskFieldHTMLData.noAttributeValueCalculator,
-    ),
-    id: new TaskFieldHTMLData(
-        'task-id',
-        TaskFieldHTMLData.noAttributeName,
-        TaskFieldHTMLData.noAttributeValueCalculator,
-    ),
-    blockLink: new TaskFieldHTMLData(
-        'task-block-link',
-        TaskFieldHTMLData.noAttributeName,
-        TaskFieldHTMLData.noAttributeValueCalculator,
-    ),
+    description: createFieldWithoutDataAttributes('task-description'),
+    recurrenceRule: createFieldWithoutDataAttributes('task-recurring'),
+    blockedBy: createFieldWithoutDataAttributes('task-blockedBy'),
+    id: createFieldWithoutDataAttributes('task-id'),
+    blockLink: createFieldWithoutDataAttributes('task-block-link'),
 };
diff --git a/src/Renderer/TaskLineRenderer.ts b/src/Renderer/TaskLineRenderer.ts
index 4e4844ed..84b02f9c 100644
--- a/src/Renderer/TaskLineRenderer.ts
+++ b/src/Renderer/TaskLineRenderer.ts
@@ -3,11 +3,10 @@ import { Component, MarkdownRenderer } from 'obsidian';
 import { GlobalFilter } from '../Config/GlobalFilter';
 import { TASK_FORMATS, getSettings } from '../Config/Settings';
 import { replaceTaskWithTasks } from '../Obsidian/File';
-import type { TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
+import type { TaskLayoutComponent, TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
 import type { QueryLayoutOptions } from '../Layout/QueryLayoutOptions';
 import type { Task } from '../Task/Task';
 import * as taskModule from '../Task/Task';
-import type { TaskLayoutComponent } from '../Layout/TaskLayout';
 import { StatusMenu } from '../ui/Menus/StatusMenu';
 import { StatusRegistry } from '../Statuses/StatusRegistry';
 import { TaskFieldRenderer } from './TaskFieldRenderer';
diff --git a/src/TaskSerializer/DataviewTaskSerializer.ts b/src/TaskSerializer/DataviewTaskSerializer.ts
index b28413f7..a11dfe49 100644
--- a/src/TaskSerializer/DataviewTaskSerializer.ts
+++ b/src/TaskSerializer/DataviewTaskSerializer.ts
@@ -1,4 +1,4 @@
-import type { TaskLayoutComponent } from '../Layout/TaskLayout';
+import type { TaskLayoutComponent } from '../Layout/TaskLayoutOptions';
 import type { Task } from '../Task/Task';
 import { Priority } from '../Task/Task';
 import { DefaultTaskSerializer } from './DefaultTaskSerializer';
diff --git a/src/TaskSerializer/DefaultTaskSerializer.ts b/src/TaskSerializer/DefaultTaskSerializer.ts
index fb3d68b2..c67457c7 100644
--- a/src/TaskSerializer/DefaultTaskSerializer.ts
+++ b/src/TaskSerializer/DefaultTaskSerializer.ts
@@ -1,6 +1,5 @@
 import type { Moment } from 'moment';
-import { TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
-import type { TaskLayoutComponent } from '../Layout/TaskLayout';
+import { type TaskLayoutComponent, TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
 import { Recurrence } from '../Task/Recurrence';
 import { Priority, Task, TaskRegularExpressions } from '../Task/Task';
 import type { TaskDetails, TaskSerializer } from '.';


Test patch:
diff --git a/tests/Layout/TaskLayout.test.ts b/tests/Layout/TaskLayout.test.ts
index 5db797ab..793ecddc 100644
--- a/tests/Layout/TaskLayout.test.ts
+++ b/tests/Layout/TaskLayout.test.ts
@@ -2,6 +2,7 @@
  * @jest-environment jsdom
  */
 
+import { QueryLayout } from '../../src/Layout/QueryLayout';
 import { TaskLayoutOptions } from '../../src/Layout/TaskLayoutOptions';
 import { QueryLayoutOptions } from '../../src/Layout/QueryLayoutOptions';
 import { TaskLayout } from '../../src/Layout/TaskLayout';
@@ -9,7 +10,10 @@ import { TaskLayout } from '../../src/Layout/TaskLayout';
 describe('TaskLayout tests', () => {
     it('should generate expected CSS classes for default layout', () => {
         const taskLayout = new TaskLayout();
-        expect(taskLayout.taskListHiddenClasses().join('\n')).toMatchInlineSnapshot('"tasks-layout-hide-urgency"');
+        const queryLayout = new QueryLayout();
+
+        const hiddenClasses = [...taskLayout.applyTaskLayoutOptions(), ...queryLayout.applyQueryLayoutOptions()];
+        expect(hiddenClasses.join('\n')).toMatchInlineSnapshot('"tasks-layout-hide-urgency"');
     });
 
     it('should generate expected CSS classes with all default options reversed', () => {
@@ -23,9 +27,11 @@ describe('TaskLayout tests', () => {
             queryLayoutOptions[key2] = !queryLayoutOptions[key2];
         });
 
-        const taskLayout = new TaskLayout(taskLayoutOptions, queryLayoutOptions);
+        const taskLayout = new TaskLayout(taskLayoutOptions);
+        const queryLayout = new QueryLayout(queryLayoutOptions);
 
-        expect(taskLayout.taskListHiddenClasses().join('\n')).toMatchInlineSnapshot(`
+        const hiddenClasses = [...taskLayout.applyTaskLayoutOptions(), ...queryLayout.applyQueryLayoutOptions()];
+        expect(hiddenClasses.join('\n')).toMatchInlineSnapshot(`
             "tasks-layout-hide-id
             tasks-layout-hide-blockedBy
             tasks-layout-hide-priority
diff --git a/tests/Renderer/TaskLineRenderer.test.ts b/tests/Renderer/TaskLineRenderer.test.ts
index 277a55aa..6e284be2 100644
--- a/tests/Renderer/TaskLineRenderer.test.ts
+++ b/tests/Renderer/TaskLineRenderer.test.ts
@@ -5,12 +5,11 @@ import moment from 'moment';
 import { DebugSettings } from '../../src/Config/DebugSettings';
 import { GlobalFilter } from '../../src/Config/GlobalFilter';
 import { resetSettings, updateSettings } from '../../src/Config/Settings';
-import { TaskLayoutOptions } from '../../src/Layout/TaskLayoutOptions';
+import { type TaskLayoutComponent, TaskLayoutOptions, taskLayoutComponents } from '../../src/Layout/TaskLayoutOptions';
 import { DateParser } from '../../src/Query/DateParser';
 import { QueryLayoutOptions } from '../../src/Layout/QueryLayoutOptions';
 import type { Task } from '../../src/Task/Task';
 import { TaskRegularExpressions } from '../../src/Task/Task';
-import { type TaskLayoutComponent, taskLayoutComponents } from '../../src/Layout/TaskLayout';
 import type { TextRenderer } from '../../src/Renderer/TaskLineRenderer';
 import { TaskLineRenderer } from '../../src/Renderer/TaskLineRenderer';
 import { fromLine } from '../TestingTools/TestHelpers';