generated from obsidianmd/obsidian-sample-plugin
-
-
Notifications
You must be signed in to change notification settings - Fork 240
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #2493 from obsidian-tasks-group/edit-status-usability
feat: Improve usability of Statuses context menu
- Loading branch information
Showing
8 changed files
with
256 additions
and
29 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(); | ||
| }); | ||
| }); |