Merge pull request obsidian-tasks-group#2442 from ilandikov/tfr-add-d…

…ata-attributes

refactor: add data attributes in `TaskFieldRenderer` instead of returning them

src/TaskFieldRenderer.ts

@@ -3,26 +3,24 @@ import { PriorityTools } from './lib/PriorityTools';
 import type { Task } from './Task';
 import type { TaskLayoutComponent } from './TaskLayout';
 
-export type AttributesDictionary = { [key: string]: string };
-
 export class TaskFieldRenderer {
  private readonly data = taskFieldHTMLData;
 
  /**
- * Searches for the component among the {@link taskFieldHTMLData} and gets its data attribute
- * in a given task. The data attribute shall be 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.
+ * Adds data attribute to an {@link element} for a component. For example,
+ * a `<span>` describing a task with medium priority and done yesterday will have
+ * `data-task-priority="medium" data-task-due="past-1d"` in its data attributes (One data attribute per component).
  *
- * If the data attribute is absent in the task, an empty {@link AttributesDictionary} is returned.
+ * If no data was found for a component in a task, data attribute won't be added.
  *
- * For detailed calculation see {@link TaskFieldHTMLData.dataAttribute}.
+ * For detailed calculation see {@link TaskFieldHTMLData.addDataAttribute}.
  *
- * @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
+ * @param element the HTML element to add the data attribute to.
+ * @param task the task from which the for the data attribute shall be taken.
+ * @param component the component of the task for which the data attribute has to be added.
  */
- public dataAttribute(component: TaskLayoutComponent, task: Task) {
- return this.data[component].dataAttribute(component, task);
+ public addDataAttribute(element: HTMLElement, task: Task, component: TaskLayoutComponent) {
+ this.data[component].addDataAttribute(element, task, component);
  }
 
  /**
@@ -107,25 +105,22 @@ export class TaskFieldHTMLData {
  }
 
  /**
- * Shall be called only by {@link TaskFieldRenderer}. Use that class if you need the data attributes.
+ * Shall be called only by {@link TaskFieldRenderer}. Use that class if you need to add a data attribute.
  *
- * @returns the data attribute, associated to with a task's component, added in the task's `<span>`.
+ * Adds the data attribute, associated to with a task's component to an HTML element.
  * 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.
+ * `data-task-priority="medium" data-task-due="past-1d" ` in its data attributes (One data attribute per component).
  *
  * Calculation of the value is done with {@link TaskFieldHTMLData.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
+ * @param element the HTML element to add the data attribute to.
+ * @param task the task from which the data shall be taken.
+ * @param component the component of the task for which the data attribute has to be added.
  */
- public dataAttribute(component: TaskLayoutComponent, task: Task) {
- const dataAttribute: AttributesDictionary = {};
-
+ public addDataAttribute(element: HTMLElement, task: Task, component: TaskLayoutComponent) {
  if (this.attributeName !== TaskFieldHTMLData.noAttributeName) {
- dataAttribute[this.attributeName] = this.attributeValueCalculator(component, task);
+ element.dataset[this.attributeName] = this.attributeValueCalculator(component, task);
  }
-
- return dataAttribute;
  }
 }
 

src/TaskLineRenderer.ts

@@ -68,11 +68,9 @@ export class TaskLineRenderer {
  }
 
  /**
- * Renders a given Task object into an HTML List Item (LI) element, using the given renderDetails
- * configuration and a supplied TextRenderer (typically the Obsidian Markdown renderer, but for testing
- * purposes it can be a simpler one).
+ * Renders a given Task object into an HTML List Item (LI) element.
  *
- * The element includes the task and its various components (description, priority, block link etc), the
+ * The element includes the task and its various components (description, priority, block link etc.), the
  * checkbox on the left with its event handling of completing the task, and the button for editing the task.
  *
  * @returns an HTML rendered List Item element (LI) for a task.
@@ -158,7 +156,7 @@ export class TaskLineRenderer {
  // Inside that text span, we are creating another internal span, that will hold the text itself.
  // This may seem redundant, and by default it indeed does nothing, but we do it to allow the CSS
  // to differentiate between the container of the text and the text itself, so it will be possible
- // to do things like surrouding only the text (rather than its whole placeholder) with a highlight
+ // to do things like surrounding only the text (rather than its whole placeholder) with a highlight
  const internalSpan = document.createElement('span');
  span.appendChild(internalSpan);
  await this.renderComponentText(internalSpan, componentString, component, task);
@@ -169,27 +167,24 @@ export class TaskLineRenderer {
  span.classList.add(...[componentClass]);
 
  // Add the component's attribute ('priority-medium', 'due-past-1d' etc.)
- const componentDataAttribute = fieldRenderer.dataAttribute(component, task);
- for (const key in componentDataAttribute) span.dataset[key] = componentDataAttribute[key];
- for (const key in componentDataAttribute) li.dataset[key] = componentDataAttribute[key];
+ fieldRenderer.addDataAttribute(span, task, component);
+ fieldRenderer.addDataAttribute(li, task, component);
  }
  }
  }
 
  // Now build classes for the hidden task components without rendering them
  for (const component of taskLayout.hiddenTaskLayoutComponents) {
- const hiddenComponentDataAttribute = fieldRenderer.dataAttribute(component, task);
- for (const key in hiddenComponentDataAttribute) li.dataset[key] = hiddenComponentDataAttribute[key];
+ fieldRenderer.addDataAttribute(li, task, component);
  }
 
- // If a task has no priority field set, its priority will not be rendered as part of the loop above and
+ // If a task has no priority field set, its priority will not be rendered as part of the loop above, and
  // it will not be set a priority data attribute.
  // In such a case we want the upper task LI element to mark the task has a 'normal' priority.
  // So if the priority was not rendered, force it through the pipe of getting the component data for the
  // priority field.
  if (li.dataset.taskPriority === undefined) {
- const priorityDataAttribute = fieldRenderer.dataAttribute('priority', task);
- for (const key in priorityDataAttribute) li.dataset[key] = priorityDataAttribute[key];
+ fieldRenderer.addDataAttribute(li, task, 'priority');
  }
  }
 
@@ -249,14 +244,14 @@ export class TaskLineRenderer {
  */
  private addInternalClasses(component: TaskLayoutComponent, internalSpan: HTMLSpanElement) {
  /*
- * Sanitize tag names so they will be valid attribute values according to the HTML spec:
+ * Sanitize tag names, so they will be valid attribute values according to the HTML spec:
  * https://html.spec.whatwg.org/multipage/parsing.html#attribute-value-(double-quoted)-state
  */
  function tagToAttributeValue(tag: string) {
  // eslint-disable-next-line no-control-regex
  const illegalChars = /["&\x00\r\n]/g;
  let sanitizedTag = tag.replace(illegalChars, '-');
- // And if after sanitazation the name starts with dashes or underscores, remove them.
+ // And if after sanitization the name starts with dashes or underscores, remove them.
  sanitizedTag = sanitizedTag.replace(/^[-_]+/, '');
  if (sanitizedTag.length > 0) return sanitizedTag;
  else return null;

tests/TaskFieldRenderer.test.ts

@@ -7,6 +7,8 @@ import { TaskBuilder } from './TestingTools/TaskBuilder';
 
 window.moment = moment;
 
+const fieldRenderer = new TaskFieldRenderer();
+
 describe('Field Layouts Container tests', () => {
  beforeEach(() => {
  jest.useFakeTimers();
@@ -17,51 +19,52 @@ describe('Field Layouts Container tests', () => {
  jest.useRealTimers();
  });
 
- it('should get the data attribute of an existing component (date)', () => {
- const container = new TaskFieldRenderer();
+ it('should add a data attribute for an existing component (date)', () => {
  const task = new TaskBuilder().dueDate('2023-11-20').build();
+ const span = document.createElement('span');
 
- const dueDateDataAttribute = container.dataAttribute('dueDate', task);
+ fieldRenderer.addDataAttribute(span, task, 'dueDate');
 
- expect(Object.keys(dueDateDataAttribute).length).toEqual(1);
- expect(dueDateDataAttribute['taskDue']).toEqual('future-1d');
+ expect(Object.keys(span.dataset).length).toEqual(1);
+ expect(span.dataset['taskDue']).toEqual('future-1d');
  });
 
- it('should get the data attribute of an existing component (not date)', () => {
- const container = new TaskFieldRenderer();
+ it('should add a data attribute for an existing component (not date)', () => {
  const task = TaskBuilder.createFullyPopulatedTask();
+ const span = document.createElement('span');
 
- const dueDateDataAttribute = container.dataAttribute('priority', task);
+ fieldRenderer.addDataAttribute(span, task, 'priority');
 
- expect(Object.keys(dueDateDataAttribute).length).toEqual(1);
- expect(dueDateDataAttribute['taskPriority']).toEqual('medium');
+ expect(Object.keys(span.dataset).length).toEqual(1);
+ expect(span.dataset['taskPriority']).toEqual('medium');
  });
-
- it('should return empty data attributes dictionary for a missing component', () => {
- const container = new TaskFieldRenderer();
+ it('should not add any data attributes for a missing component', () => {
  const task = new TaskBuilder().build();
+ const span = document.createElement('span');
 
- const dueDateDataAttribute = container.dataAttribute('recurrenceRule', task);
+ fieldRenderer.addDataAttribute(span, task, 'recurrenceRule');
 
- expect(Object.keys(dueDateDataAttribute).length).toEqual(0);
+ expect(Object.keys(span.dataset).length).toEqual(0);
  });
 });
 
 describe('Field Layout Detail tests', () => {
- it('should supply a class name and a data attribute name', () => {
+ it('should supply a class name', () => {
  const fieldLayoutDetail = new TaskFieldHTMLData('stuff', 'taskAttribute', () => {
  return '';
  });
  expect(fieldLayoutDetail.className).toEqual('stuff');
  });
 
- it('should return a data attribute', () => {
+ it('should add a data attribute for an HTML element', () => {
  const fieldLayoutDetail = new TaskFieldHTMLData('dataAttributeTest', 'aKey', () => {
  return 'aValue';
  });
- const dataAttribute = fieldLayoutDetail.dataAttribute('description', new TaskBuilder().build());
+ const span = document.createElement('span');
+
+ fieldLayoutDetail.addDataAttribute(span, new TaskBuilder().build(), 'description');
 
- expect(Object.keys(dataAttribute).length).toEqual(1);
- expect(dataAttribute['aKey']).toEqual('aValue');
+ expect(Object.keys(span.dataset).length).toEqual(1);
+ expect(span.dataset['aKey']).toEqual('aValue');
  });
 });

tests/TaskLineRenderer.test.ts

@@ -8,7 +8,7 @@ import { resetSettings, updateSettings } from '../src/Config/Settings';
 import { DateParser } from '../src/Query/DateParser';
 import type { Task } from '../src/Task';
 import { TaskRegularExpressions } from '../src/Task';
-import { type AttributesDictionary, TaskFieldRenderer } from '../src/TaskFieldRenderer';
+import { TaskFieldRenderer } from '../src/TaskFieldRenderer';
 import { LayoutOptions } from '../src/TaskLayout';
 import type { TextRenderer } from '../src/TaskLineRenderer';
 import { TaskLineRenderer } from '../src/TaskLineRenderer';
@@ -19,6 +19,8 @@ import { TaskBuilder } from './TestingTools/TaskBuilder';
 jest.mock('obsidian');
 window.moment = moment;
 
+type AttributesDictionary = { [key: string]: string };
+
 const fieldRenderer = new TaskFieldRenderer();
 
 /**