Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2408 (2023-11-10 11:06:58)

refactor: move to FieldLayoutDetail and linline 2 functions
# Description

Following #2403 now the getTaskComponentClass() and getComponentDataAttribute() from TaskLineRenderer.ts shall be refactored.

## Motivation and Context

Move behaviour towards the class that shall have it.

## How has this been tested?

Unit tests.

## 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)


ilandikov: I have not run the smoke tests yet. Thanks to the approval tests we have set up together, I think we can be pretty confident that this will pass, so I propose that I run the smoke tests after your review.

Patch:
diff --git a/src/TaskLineRenderer.ts b/src/TaskLineRenderer.ts
index 0e863b94..709c88fd 100644
--- a/src/TaskLineRenderer.ts
+++ b/src/TaskLineRenderer.ts
@@ -22,9 +22,9 @@ export type TaskLineRenderDetails = {
 type AttributeValueCalculator = (component: TaskLayoutComponent, task: Task) => string;
 
 export class FieldLayoutDetail {
-    className: string;
-    attributeName: string;
-    attributeValueCalculator: AttributeValueCalculator;
+    readonly className: string;
+    readonly attributeName: string;
+    readonly attributeValueCalculator: AttributeValueCalculator;
 
     public static noAttributeName = '';
     public static noAttributeValueCalculator: AttributeValueCalculator = () => {
@@ -32,9 +32,11 @@ export class FieldLayoutDetail {
     };
 
     /**
-     * @param className CSS class of the component.
+     * @param className CSS class that describes what the component is, e.g. a due date or a priority.
+     *
      * @param attributeName if the component needs data attribute (`data-key="value"`) this is the key.
      * Otherwise, set this to {@link FieldLayoutDetail.noAttributeName}.
+     *
      * @param attributeValueCalculator And this is the value.
      * Set to {@link FieldLayoutDetail.noAttributeValueCalculator} if shall be empty.
      *
@@ -52,6 +54,26 @@ export class FieldLayoutDetail {
         this.attributeName = attributeName;
         this.attributeValueCalculator = attributeValueCalculator;
     }
+
+    /**
+     * @returns the data attribute, associated to with a task's component, added in the task's `<span>`.
+     * For example, a task with medium priority and done yesterday will have
+     * `data-task-priority="medium" data-task-due="past-1d" ` in its data attributes.
+     *
+     * Calculation of the value is done with {@link FieldLayoutDetail.attributeValueCalculator}.
+     *
+     * @param component the component of the task for which the data attribute has to be generated.
+     * @param task the task from which the data shall be taken
+     */
+    public getDataAttribute(component: TaskLayoutComponent, task: Task) {
+        const dataAttribute: AttributesDictionary = {};
+
+        if (this.attributeName !== FieldLayoutDetail.noAttributeName) {
+            dataAttribute[this.attributeName] = this.attributeValueCalculator(component, task);
+        }
+
+        return dataAttribute;
+    }
 }
 
 const dateDataAttributeCalculator: AttributeValueCalculator = (component: TaskLayoutComponent, task: Task) => {
@@ -217,11 +239,14 @@ async function taskToHtml(
                 addInternalClasses(component, internalSpan);
 
                 // Add the component's CSS class describing what this component is (priority, due date etc.)
-                const componentClass = getTaskComponentClass(component);
-                span.classList.add(...componentClass);
+                const fieldLayoutDetails = FieldLayouts[component];
+                if (fieldLayoutDetails) {
+                    const componentClass = [fieldLayoutDetails.className];
+                    span.classList.add(...componentClass);
+                }
 
                 // Add the component's attribute ('priority-medium', 'due-past-1d' etc.)
-                const componentDataAttribute = getComponentDataAttribute(component, task);
+                const componentDataAttribute = FieldLayouts[component].getDataAttribute(component, task);
                 for (const key in componentDataAttribute) span.dataset[key] = componentDataAttribute[key];
                 allAttributes = { ...allAttributes, ...componentDataAttribute };
             }
@@ -230,7 +255,7 @@ async function taskToHtml(
 
     // Now build classes for the hidden task components without rendering them
     for (const component of taskLayout.hiddenTaskLayoutComponents) {
-        const hiddenComponentDataAttribute = getComponentDataAttribute(component, task);
+        const hiddenComponentDataAttribute = FieldLayouts[component].getDataAttribute(component, task);
         allAttributes = { ...allAttributes, ...hiddenComponentDataAttribute };
     }
 
@@ -240,7 +265,7 @@ async function taskToHtml(
     // So if the priority was not rendered, force it through the pipe of getting the component data for the
     // priority field.
     if (allAttributes.taskPriority === undefined) {
-        const priorityDataAttribute = getComponentDataAttribute('priority', task);
+        const priorityDataAttribute = FieldLayouts['priority'].getDataAttribute('priority', task);
         allAttributes = { ...allAttributes, ...priorityDataAttribute };
     }
 
@@ -297,39 +322,6 @@ async function renderComponentText(
     }
 }
 
-/**
- * The CSS class that describes what the component is, e.g. a due date or a priority, and is a value from FieldLayouts.
- *
- * **Important**: The caller is responsible for ensuring that the Task being rendered does actually have a value for this field,
- * that is, that `task[component]` has a value. Only call this if task has this value.
- */
-function getTaskComponentClass(component: TaskLayoutComponent) {
-    const componentClassContainer: string[] = [];
-
-    const fieldLayoutDetail = FieldLayouts[component];
-    componentClassContainer.push(fieldLayoutDetail.className);
-
-    return componentClassContainer;
-}
-
-/**
- * The data attribute describes the content of the component, e.g. `data-task-priority="medium"`, `data-task-due="past-1d"` etc.
- */
-function getComponentDataAttribute(component: TaskLayoutComponent, task: Task) {
-    const dataAttribute: AttributesDictionary = {};
-
-    // If a TaskLayoutComponent needs a data attribute in the task's <span>, get the data attribute name (key) &
-    // data attribute value (value). Otherwise, just leave an empty string ('') as the value.
-    // The value is calculated based on FieldLayoutDetail.attributeValueCalculator
-    const fieldLayoutDetail = FieldLayouts[component];
-    const attributeName = fieldLayoutDetail.attributeName;
-    if (attributeName !== FieldLayoutDetail.noAttributeName) {
-        dataAttribute[attributeName] = fieldLayoutDetail.attributeValueCalculator(component, task);
-    }
-
-    return dataAttribute;
-}
-
 /*
  * Adds internal classes for various components (right now just tags actually), meaning that we modify the existing
  * rendered element to add classes inside it.


Test patch:
diff --git a/tests/TaskLineRenderer.test.ts b/tests/TaskLineRenderer.test.ts
index 6ca54413..ede72d6d 100644
--- a/tests/TaskLineRenderer.test.ts
+++ b/tests/TaskLineRenderer.test.ts
@@ -44,8 +44,7 @@ async function createMockParentAndRender(task: Task, layoutOptions?: LayoutOptio
 
 function getTextSpan(parentElement: HTMLElement) {
     const li = parentElement.children[0];
-    const textSpan = li.children[1] as HTMLSpanElement;
-    return textSpan;
+    return li.children[1] as HTMLSpanElement;
 }
 
 function getDescriptionText(parentElement: HTMLElement) {
@@ -695,4 +694,14 @@ describe('Field Layout Detail tests', () => {
         const attributeValue = fieldLayoutDetail.attributeValueCalculator('createdDate', new TaskBuilder().build());
         expect(attributeValue).toEqual('someValue');
     });
+
+    it('should return a data attribute', () => {
+        const fieldLayoutDetail = new FieldLayoutDetail('dataAttributeTest', 'aKey', () => {
+            return 'aValue';
+        });
+        const dataAttribute = fieldLayoutDetail.getDataAttribute('description', new TaskBuilder().build());
+
+        expect(Object.keys(dataAttribute).length).toEqual(1);
+        expect(dataAttribute['aKey']).toEqual('aValue');
+    });
 });