Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2493 (2023-12-10 15:31:14)

feat: Improve usability of Statuses context menu
<!--- Provide a general summary of your changes in the Title above -->

# Description

## Visible to users:

#### Features:

- Improve the labels shown on the status menu: they now show the symbol
- Add checkmark against the current task status
- Put a tooltip on the task checkbox, to show that there are now right-click options

![image](https://github.com/obsidian-tasks-group/obsidian-tasks/assets/4840096/603cd590-5459-4034-b488-fe92be9dc0b9)

![image](https://github.com/obsidian-tasks-group/obsidian-tasks/assets/4840096/236f115d-12ba-4e3d-afa0-96b4c364014d)

#### Fixes:

- Don't edit a task if it already has the current status
- Status menu no longer shows duplicate statuses, and empty statuses (created with 'Add New Task Status' and then not edited)

### Internal

- Move the status menu code to new class `StatusMenu` in `src/ui/Menus/StatusMenu.ts`
- Add tests for most of the behaviour of the status menu code
- Fix some async/await warnings

## Motivation and Context

- Move code out of TaskLineRenderer, as I want to add more context menus
- Add tests

## How has this been tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->

- New tests
- Extensive exploratory testing

## Screenshots (if appropriate)

See above.

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)
- [x] **New feature** (prefix: `feat` - non-breaking change which adds functionality)
  - Usability improvements

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

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [x] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://publish.obsidian.md/tasks-contributing/Documentation/About+Documentation) accordingly.
- [ ] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://publish.obsidian.md/tasks-contributing) and
[Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md).
Put an x in the boxes to confirm you agree.
-->

- [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/TaskLineRenderer.ts b/src/TaskLineRenderer.ts
index 84bfe9da..b942565b 100644
--- a/src/TaskLineRenderer.ts
+++ b/src/TaskLineRenderer.ts
@@ -1,14 +1,15 @@
 import type { Moment } from 'moment';
-import { Component, MarkdownRenderer, Menu, MenuItem } from 'obsidian';
+import { Component, MarkdownRenderer } from 'obsidian';
 import { GlobalFilter } from './Config/GlobalFilter';
 import { TASK_FORMATS, getSettings } from './Config/Settings';
 import { replaceTaskWithTasks } from './File';
-import { StatusRegistry } from './StatusRegistry';
 import type { Task } from './Task';
 import * as taskModule from './Task';
 import { TaskFieldRenderer } from './TaskFieldRenderer';
 import type { LayoutOptions, TaskLayoutComponent } from './TaskLayout';
 import { TaskLayout } from './TaskLayout';
+import { StatusMenu } from './ui/Menus/StatusMenu';
+import { StatusRegistry } from './StatusRegistry';
 
 const fieldRenderer = new TaskFieldRenderer();
 
@@ -124,31 +125,11 @@ export class TaskLineRenderer {
             });
         });
 
-        checkbox.addEventListener('contextmenu', async (ev: MouseEvent) => {
-            const menu = new Menu();
-            const commonTitle = 'Change status to: ';
-
-            const getMenuItemCallback = (item: MenuItem, statusName: string, newStatusSymbol: string) => {
-                item.setTitle(`${commonTitle}  ${statusName}`).onClick(() => {
-                    const status = StatusRegistry.getInstance().bySymbol(newStatusSymbol);
-                    const newTask = task.handleStatusChangeFromContextMenuWithRecurrenceInUsersOrder(status);
-                    replaceTaskWithTasks({
-                        originalTask: task,
-                        newTasks: newTask,
-                    });
-                });
-            };
-
-            const { statusSettings } = getSettings();
-            for (const status of statusSettings.coreStatuses) {
-                menu.addItem((item) => getMenuItemCallback(item, status.name, status.symbol));
-            }
-            for (const status of statusSettings.customStatuses) {
-                menu.addItem((item) => getMenuItemCallback(item, status.name, status.symbol));
-            }
-
+        checkbox.addEventListener('contextmenu', (ev: MouseEvent) => {
+            const menu = new StatusMenu(StatusRegistry.getInstance(), task);
             menu.showAtPosition({ x: ev.clientX, y: ev.clientY });
         });
+        checkbox.setAttribute('title', 'Right-click for options');
 
         li.prepend(checkbox);
 
diff --git a/src/ui/Menus/StatusMenu.ts b/src/ui/Menus/StatusMenu.ts
new file mode 100644
index 00000000..eab9f1e4
--- /dev/null
+++ b/src/ui/Menus/StatusMenu.ts
@@ -0,0 +1,78 @@
+import { Menu, MenuItem } from 'obsidian';
+import type { StatusRegistry } from '../../StatusRegistry';
+import { replaceTaskWithTasks } from '../../File';
+import type { Task } from '../../Task';
+import { StatusSettings } from '../../Config/StatusSettings';
+
+/**
+ * A function for replacing one task with zero or more new tasks.
+ * @see {@link defaultTaskSaver}
+ */
+type TaskSaver = (originalTask: Task, newTasks: Task | Task[]) => Promise<void>;
+
+/**
+ * A default implementation of {@link TaskSaver} that calls {@link replaceTaskWithTasks}
+ * @param originalTask
+ * @param newTasks
+ */
+async function defaultTaskSaver(originalTask: Task, newTasks: Task | Task[]) {
+    await replaceTaskWithTasks({
+        originalTask,
+        newTasks,
+    });
+}
+
+/**
+ * A Menu of options for editing the status of a Task object.
+ *
+ * @example
+ *     checkbox.addEventListener('contextmenu', (ev: MouseEvent) => {
+ *         const menu = new StatusMenu(StatusRegistry.getInstance(), task);
+ *         menu.showAtPosition({ x: ev.clientX, y: ev.clientY });
+ *     });
+ *     checkbox.setAttribute('title', 'Right-click for options');
+ */
+export class StatusMenu extends Menu {
+    private statusRegistry: StatusRegistry;
+    private readonly taskSaver: TaskSaver;
+
+    /**
+     * Constructor, which sets up the menu items.
+     * @param statusRegistry - the statuses to be shown in the menu.
+     * @param task - the Task to be edited.
+     * @param taskSaver - an optional {@link TaskSaver} function. If not supplied, {@link replaceTaskWithTasks}
+     *                    will be used to update the file containing the Task.
+     *                    An alternative implementation can be used in tests.
+     */
+    constructor(statusRegistry: StatusRegistry, task: Task, taskSaver: TaskSaver = defaultTaskSaver) {
+        super();
+
+        this.statusRegistry = statusRegistry;
+        this.taskSaver = taskSaver;
+
+        const commonTitle = 'Change status to:';
+
+        const getMenuItemCallback = (task: Task, item: MenuItem, statusName: string, newStatusSymbol: string) => {
+            const title = `${commonTitle} [${newStatusSymbol}] ${statusName}`;
+            item.setTitle(title)
+                .setChecked(newStatusSymbol === task.status.symbol)
+                .onClick(async () => {
+                    if (newStatusSymbol !== task.status.symbol) {
+                        const status = this.statusRegistry.bySymbol(newStatusSymbol);
+                        const newTask = task.handleStatusChangeFromContextMenuWithRecurrenceInUsersOrder(status);
+                        await this.taskSaver(task, newTask);
+                    }
+                });
+        };
+
+        const coreStatuses = new StatusSettings().coreStatuses.map((setting) => setting.symbol);
+        // Put the core statuses at the top of the menu:
+        for (const matchCoreTask of [true, false]) {
+            for (const status of statusRegistry.registeredStatuses) {
+                if (coreStatuses.includes(status.symbol) === matchCoreTask) {
+                    this.addItem((item) => getMenuItemCallback(task, item, status.name, status.symbol));
+                }
+            }
+        }
+    }
+}


Test patch:
diff --git a/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_full_mode.approved.html b/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_full_mode.approved.html
index b708399f..df069893 100644
--- a/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_full_mode.approved.html
+++ b/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_full_mode.approved.html
@@ -12,7 +12,7 @@
     data-task=""
     data-line="0"
     data-task-status-name="Todo"
-    data-task-status-type="TODO"><input class="task-list-item-checkbox" type="checkbox"
+    data-task-status-type="TODO"><input class="task-list-item-checkbox" type="checkbox" title="Right-click for options"
     data-line="0">
         <span class="tasks-list-text">
         <span class="task-description">
diff --git a/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_short_mode.approved.html b/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_short_mode.approved.html
index 30a788cc..de93c26b 100644
--- a/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_short_mode.approved.html
+++ b/tests/TaskLineRenderer.test.Visualise_HTML_Full_task_-_short_mode.approved.html
@@ -12,7 +12,7 @@
     data-task=""
     data-line="0"
     data-task-status-name="Todo"
-    data-task-status-type="TODO"><input class="task-list-item-checkbox" type="checkbox"
+    data-task-status-type="TODO"><input class="task-list-item-checkbox" type="checkbox" title="Right-click for options"
     data-line="0">
         <span class="tasks-list-text">
         <span class="task-description">
diff --git a/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_full_mode.approved.html b/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_full_mode.approved.html
index b52c5041..4799c8bf 100644
--- a/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_full_mode.approved.html
+++ b/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_full_mode.approved.html
@@ -7,7 +7,7 @@
     data-task="-"
     data-line="0"
     data-task-status-name="Cancelled"
-    data-task-status-type="CANCELLED"><input class="task-list-item-checkbox" type="checkbox"
+    data-task-status-type="CANCELLED"><input class="task-list-item-checkbox" type="checkbox" title="Right-click for options"
     data-line="0">
         <span class="tasks-list-text">
         <span class="task-description">
diff --git a/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_short_mode.approved.html b/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_short_mode.approved.html
index b52c5041..4799c8bf 100644
--- a/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_short_mode.approved.html
+++ b/tests/TaskLineRenderer.test.Visualise_HTML_Minimal_task_-_short_mode.approved.html
@@ -7,7 +7,7 @@
     data-task="-"
     data-line="0"
     data-task-status-name="Cancelled"
-    data-task-status-type="CANCELLED"><input class="task-list-item-checkbox" type="checkbox"
+    data-task-status-type="CANCELLED"><input class="task-list-item-checkbox" type="checkbox" title="Right-click for options"
     data-line="0">
         <span class="tasks-list-text">
         <span class="task-description">
diff --git a/tests/__mocks__/obsidian.ts b/tests/__mocks__/obsidian.ts
index cb0ff5c3..669af806 100644
--- a/tests/__mocks__/obsidian.ts
+++ b/tests/__mocks__/obsidian.ts
@@ -1,1 +1,40 @@
 export {};
+
+export class MenuItem {
+    public title: string | DocumentFragment = '';
+    public callback: (evt: MouseEvent | KeyboardEvent) => any;
+    public checked = false;
+
+    constructor() {
+        this.callback = (_evt: MouseEvent | KeyboardEvent) => console.log('callback not defined');
+    }
+
+    public setTitle(title: string | DocumentFragment): this {
+        this.title = title;
+        return this;
+    }
+
+    public onClick(callback: (evt: MouseEvent | KeyboardEvent) => any): this {
+        this.callback = callback;
+        return this;
+    }
+    public setChecked(checked: boolean | null): this {
+        this.checked = checked ? checked : false;
+        return this;
+    }
+}
+
+export class Menu {
+    public items: MenuItem[] = [];
+
+    /**
+     * Adds a menu item. Only works when menu is not shown yet.
+     * @public
+     */
+    addItem(cb: (item: MenuItem) => any): this {
+        const item = new MenuItem();
+        cb(item);
+        this.items.push(item);
+        return this;
+    }
+}
diff --git a/tests/ui/Menus/StatusMenu.test.ts b/tests/ui/Menus/StatusMenu.test.ts
new file mode 100644
index 00000000..f2463e14
--- /dev/null
+++ b/tests/ui/Menus/StatusMenu.test.ts
@@ -0,0 +1,129 @@
+import { StatusMenu } from '../../../src/ui/Menus/StatusMenu';
+import { TaskBuilder } from '../../TestingTools/TaskBuilder';
+import type { MenuItem } from '../../__mocks__/obsidian';
+import { StatusRegistry } from '../../../src/StatusRegistry';
+import { StatusSettings } from '../../../src/Config/StatusSettings';
+import { resetSettings, updateSettings } from '../../../src/Config/Settings';
+import { StatusConfiguration, StatusType } from '../../../src/StatusConfiguration';
+import { Status } from '../../../src/Status';
+import type { Task } from '../../../src/Task';
+
+export {};
+
+afterEach(() => {
+    resetSettings();
+});
+
+function menuToString(menu: StatusMenu) {
+    // @ts-expect-error TS2339: Property 'items' does not exist on type 'StatusMenu'.
+    const items: MenuItem[] = menu.items;
+    return '\n' + items.map((item) => `${item.checked ? 'x' : ' '} ${item.title}`).join('\n');
+}
+
+describe('StatusMenu', () => {
+    let taskBeingOverwritten: Task | undefined;
+    let tasksBeingSaved: Task[] | undefined;
+
+    async function testableTaskSaver(originalTask: Task, newTasks: Task | Task[]) {
+        taskBeingOverwritten = originalTask;
+        tasksBeingSaved = Array.isArray(newTasks) ? newTasks : [newTasks];
+    }
+
+    beforeEach(() => {
+        taskBeingOverwritten = undefined;
+        tasksBeingSaved = undefined;
+    });
+
+    it('should show checkmark against the current task status', () => {
+        // Arrange
+        const task = new TaskBuilder().status(Status.makeInProgress()).build();
+        const statusRegistry = new StatusRegistry();
+
+        // Act
+        const menu = new StatusMenu(statusRegistry, task);
+
+        // Assert
+        const itemsAsText = menuToString(menu);
+        expect(itemsAsText).toMatchInlineSnapshot(`
+            "
+              Change status to: [ ] Todo
+              Change status to: [x] Done
+            x Change status to: [/] In Progress
+              Change status to: [-] Cancelled"
+        `);
+    });
+
+    it('should ignore duplicate status symbols in global status settings', () => {
+        // Arrange
+        const statusSettings = new StatusSettings();
+        statusSettings.customStatuses.push(new StatusConfiguration('%', '% 1', '&', false, StatusType.TODO));
+        statusSettings.customStatuses.push(new StatusConfiguration('%', '% 2', '&', false, StatusType.TODO));
+        updateSettings({
+            statusSettings: statusSettings,
+        });
+
+        const statusRegistry = new StatusRegistry();
+        StatusSettings.applyToStatusRegistry(statusSettings, statusRegistry);
+
+        const task = new TaskBuilder().build();
+
+        // Act
+        const menu = new StatusMenu(statusRegistry, task);
+
+        // Assert
+        const itemsAsText = menuToString(menu);
+        expect(itemsAsText).toMatchInlineSnapshot(`
+            "
+            x Change status to: [ ] Todo
+              Change status to: [x] Done
+              Change status to: [/] In Progress
+              Change status to: [-] Cancelled
+              Change status to: [%] % 1"
+        `);
+    });
+
+    it('should modify task, if different status selected', () => {
+        // Arrange
+        const onlyShowCancelled = new StatusRegistry();
+        onlyShowCancelled.clearStatuses();
+        onlyShowCancelled.add(Status.makeCancelled());
+
+        const task = new TaskBuilder().status(Status.makeTodo()).build();
+        const menu = new StatusMenu(onlyShowCancelled, task, testableTaskSaver);
+
+        // Act
+        // @ts-expect-error TS2339: Property 'items' does not exist on type 'StatusMenu'.
+        const todoItem = menu.items[0];
+        todoItem.callback();
+
+        // Assert
+        expect(taskBeingOverwritten).not.toBeUndefined();
+        expect(Object.is(task, taskBeingOverwritten)).toEqual(true);
+        expect(taskBeingOverwritten!.status.symbol).toEqual(' ');
+
+        expect(tasksBeingSaved).not.toBeUndefined();
+        expect(tasksBeingSaved!.length).toEqual(1);
+        expect(tasksBeingSaved![0].status.symbol).toEqual('-');
+    });
+
+    it('should not modify task, if current status selected', () => {
+        // Arrange
+        const task = new TaskBuilder().build();
+        const statusRegistry = new StatusRegistry();
+
+        // Act
+        const menu = new StatusMenu(statusRegistry, task, testableTaskSaver);
+
+        // Act
+        // @ts-expect-error TS2339: Property 'items' does not exist on type 'StatusMenu'.
+        const todoItem = menu.items[0];
+        expect(todoItem.title).toEqual('Change status to: [ ] Todo');
+        todoItem.callback();
+
+        // Assert
+        // testableTaskSaver() should never have been called, so the values
+        // it saves should still be undefined:
+        expect(taskBeingOverwritten).toBeUndefined();
+        expect(tasksBeingSaved).toBeUndefined();
+    });
+});