From acedc67f2af1d5a86df910a12bf5d3efaec7a1e6 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sun, 4 Jan 2026 17:07:29 +0100 Subject: [PATCH] fix(schedule): start tracking selected task when pressing Y in schedule view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user clicked on a task in the Schedule view and pressed Y to start time tracking, a different task was being tracked instead. This happened because Schedule view only sets selectedTaskId (for the detail panel) but not focusedTaskId (used by keyboard shortcuts). The fix modifies TaskShortcutService to use selectedTaskId as a fallback when focusedTaskId is null, specifically for the togglePlay (Y) shortcut. Priority order: 1. If focusedTaskId exists → delegate to TaskComponent (existing behavior) 2. If no focusedTaskId but selectedTaskId exists → toggle tracking for selected task 3. If neither exists → use global toggle behavior Fixes #5884 --- .../tasks/task-shortcut.service.spec.ts | 269 ++++++++++++++++++ .../features/tasks/task-shortcut.service.ts | 59 ++-- 2 files changed, 305 insertions(+), 23 deletions(-) create mode 100644 src/app/features/tasks/task-shortcut.service.spec.ts diff --git a/src/app/features/tasks/task-shortcut.service.spec.ts b/src/app/features/tasks/task-shortcut.service.spec.ts new file mode 100644 index 000000000..689ad87f9 --- /dev/null +++ b/src/app/features/tasks/task-shortcut.service.spec.ts @@ -0,0 +1,269 @@ +import { TestBed } from '@angular/core/testing'; +import { TaskShortcutService } from './task-shortcut.service'; +import { TaskFocusService } from './task-focus.service'; +import { TaskService } from './task.service'; +import { GlobalConfigService } from '../config/global-config.service'; +import { signal } from '@angular/core'; + +describe('TaskShortcutService', () => { + let service: TaskShortcutService; + let mockTaskFocusService: { + focusedTaskId: ReturnType>; + lastFocusedTaskComponent: ReturnType>; + }; + let mockTaskService: jasmine.SpyObj & { + selectedTaskId: ReturnType>; + currentTaskId: ReturnType>; + }; + let mockConfigService: { + cfg: ReturnType>; + }; + + const defaultKeyboardConfig = { + togglePlay: 'Y', + taskEditTitle: 'Enter', + taskToggleDetailPanelOpen: 'I', + taskOpenEstimationDialog: 'T', + taskSchedule: 'S', + taskToggleDone: 'D', + taskAddSubTask: 'A', + taskAddAttachment: null, + taskDelete: 'Backspace', + taskMoveToProject: 'P', + taskEditTags: 'G', + taskOpenContextMenu: null, + moveToBacklog: 'B', + moveToTodaysTasks: 'F', + selectPreviousTask: 'K', + selectNextTask: 'J', + collapseSubTasks: 'H', + expandSubTasks: 'L', + moveTaskUp: null, + moveTaskDown: null, + moveTaskToTop: null, + moveTaskToBottom: null, + }; + + const createKeyboardEvent = (key: string, code?: string): KeyboardEvent => { + return new KeyboardEvent('keydown', { + key, + code: code || (key.length === 1 ? `Key${key.toUpperCase()}` : key), + bubbles: true, + cancelable: true, + }); + }; + + beforeEach(() => { + // Create signal-based mocks + mockTaskFocusService = { + focusedTaskId: signal(null), + lastFocusedTaskComponent: signal(null), + }; + + mockTaskService = { + selectedTaskId: signal(null), + currentTaskId: signal(null), + setCurrentId: jasmine.createSpy('setCurrentId'), + toggleStartTask: jasmine.createSpy('toggleStartTask'), + } as any; + + mockConfigService = { + cfg: signal({ + keyboard: defaultKeyboardConfig, + appFeatures: { + isTimeTrackingEnabled: true, + }, + }), + }; + + TestBed.configureTestingModule({ + providers: [ + TaskShortcutService, + { provide: TaskFocusService, useValue: mockTaskFocusService }, + { provide: TaskService, useValue: mockTaskService }, + { provide: GlobalConfigService, useValue: mockConfigService }, + ], + }); + + service = TestBed.inject(TaskShortcutService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + describe('handleTaskShortcuts - togglePlay (Y key)', () => { + describe('when focused task exists', () => { + it('should delegate to focused task component togglePlayPause method', () => { + // Arrange + const mockTaskComponent = { + togglePlayPause: jasmine.createSpy('togglePlayPause'), + taskContextMenu: () => undefined, // No context menu open + }; + mockTaskFocusService.focusedTaskId.set('focused-task-1'); + mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(true); + expect(event.preventDefault).toHaveBeenCalled(); + expect(mockTaskComponent.togglePlayPause).toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).not.toHaveBeenCalled(); + expect(mockTaskService.toggleStartTask).not.toHaveBeenCalled(); + }); + }); + + describe('when no focused task but selected task exists', () => { + it('should start tracking selected task when not currently tracking it', () => { + // Arrange + mockTaskFocusService.focusedTaskId.set(null); + mockTaskService.selectedTaskId.set('selected-task-1'); + mockTaskService.currentTaskId.set(null); + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(true); + expect(event.preventDefault).toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).toHaveBeenCalledWith('selected-task-1'); + expect(mockTaskService.toggleStartTask).not.toHaveBeenCalled(); + }); + + it('should start tracking selected task when tracking a different task', () => { + // Arrange + mockTaskFocusService.focusedTaskId.set(null); + mockTaskService.selectedTaskId.set('selected-task-1'); + mockTaskService.currentTaskId.set('other-task'); + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(true); + expect(event.preventDefault).toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).toHaveBeenCalledWith('selected-task-1'); + }); + + it('should stop tracking when selected task is already being tracked', () => { + // Arrange + mockTaskFocusService.focusedTaskId.set(null); + mockTaskService.selectedTaskId.set('selected-task-1'); + mockTaskService.currentTaskId.set('selected-task-1'); + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(true); + expect(event.preventDefault).toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).toHaveBeenCalledWith(null); + }); + }); + + describe('when neither focused nor selected task exists', () => { + it('should use global toggle behavior', () => { + // Arrange + mockTaskFocusService.focusedTaskId.set(null); + mockTaskService.selectedTaskId.set(null); + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(true); + expect(event.preventDefault).toHaveBeenCalled(); + expect(mockTaskService.toggleStartTask).toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).not.toHaveBeenCalled(); + }); + }); + + describe('when time tracking is disabled', () => { + it('should not handle Y key when time tracking is disabled', () => { + // Arrange + mockConfigService.cfg.set({ + keyboard: defaultKeyboardConfig, + appFeatures: { + isTimeTrackingEnabled: false, + }, + }); + mockTaskFocusService.focusedTaskId.set(null); + mockTaskService.selectedTaskId.set('selected-task-1'); + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(false); + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).not.toHaveBeenCalled(); + expect(mockTaskService.toggleStartTask).not.toHaveBeenCalled(); + }); + }); + + describe('priority: focused task takes priority over selected task', () => { + it('should use focused task even when selected task is different', () => { + // Arrange + const mockTaskComponent = { + togglePlayPause: jasmine.createSpy('togglePlayPause'), + taskContextMenu: () => undefined, // No context menu open + }; + mockTaskFocusService.focusedTaskId.set('focused-task-1'); + mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); + mockTaskService.selectedTaskId.set('selected-task-2'); // Different task selected + + const event = createKeyboardEvent('Y'); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(true); + expect(mockTaskComponent.togglePlayPause).toHaveBeenCalled(); + expect(mockTaskService.setCurrentId).not.toHaveBeenCalled(); + }); + }); + }); + + describe('other shortcuts require focused task', () => { + it('should return false for non-togglePlay shortcuts when no focused task', () => { + // Arrange + mockTaskFocusService.focusedTaskId.set(null); + + // Various other shortcut keys + const otherKeys = ['D', 'Enter', 'ArrowUp', 'ArrowDown']; + + for (const key of otherKeys) { + const event = createKeyboardEvent(key); + spyOn(event, 'preventDefault'); + + // Act + const result = service.handleTaskShortcuts(event); + + // Assert + expect(result).toBe(false); + } + }); + }); +}); diff --git a/src/app/features/tasks/task-shortcut.service.ts b/src/app/features/tasks/task-shortcut.service.ts index 08574e4d3..f3810ec16 100644 --- a/src/app/features/tasks/task-shortcut.service.ts +++ b/src/app/features/tasks/task-shortcut.service.ts @@ -1,5 +1,6 @@ import { computed, inject, Injectable } from '@angular/core'; import { TaskFocusService } from './task-focus.service'; +import { TaskService } from './task.service'; import { GlobalConfigService } from '../config/global-config.service'; import { checkKeyCombo } from '../../util/check-key-combo'; import { Log } from '../../core/log'; @@ -35,6 +36,7 @@ type TaskComponentMethod = keyof TaskComponent; }) export class TaskShortcutService { private readonly _taskFocusService = inject(TaskFocusService); + private readonly _taskService = inject(TaskService); private readonly _configService = inject(GlobalConfigService); readonly isTimeTrackingEnabled = computed( () => this._configService.cfg()?.appFeatures.isTimeTrackingEnabled, @@ -47,26 +49,44 @@ export class TaskShortcutService { * @returns True if the shortcut was handled, false otherwise */ handleTaskShortcuts(ev: KeyboardEvent): boolean { - // Handle task-specific shortcuts if a task is focused - const focusedTaskId: TaskId | null = this._taskFocusService.focusedTaskId(); - - // Log.log('TaskShortcutService.handleTaskShortcuts', { - // focusedTaskId, - // key: ev.key, - // ctrlKey: ev.ctrlKey, - // shiftKey: ev.shiftKey, - // altKey: ev.altKey, - // }); - - if (!focusedTaskId) { - // Log.log('TaskShortcutService: No focused task ID'); - return false; - } - const cfg = this._configService.cfg(); if (!cfg) return false; const keys = cfg.keyboard; + const focusedTaskId: TaskId | null = this._taskFocusService.focusedTaskId(); + + // Handle togglePlay specially - it works with focusedTaskId OR selectedTaskId + // This allows starting time tracking from Schedule view where tasks are selected but not focused + if (checkKeyCombo(ev, keys.togglePlay) && this.isTimeTrackingEnabled()) { + if (focusedTaskId) { + // Focused task exists - delegate to the task component + this._handleTaskShortcut(focusedTaskId, 'togglePlayPause'); + } else { + // No focused task - check for selected task (e.g., from Schedule view) + const selectedId = this._taskService.selectedTaskId(); + if (selectedId) { + const currentTaskId = this._taskService.currentTaskId(); + if (currentTaskId === selectedId) { + // Already tracking this task - stop tracking + this._taskService.setCurrentId(null); + } else { + // Start tracking the selected task + this._taskService.setCurrentId(selectedId); + } + } else { + // Neither focused nor selected - use global toggle + this._taskService.toggleStartTask(); + } + } + ev.preventDefault(); + return true; + } + + // All other shortcuts require a focused task + if (!focusedTaskId) { + return false; + } + const isShiftOrCtrlPressed = ev.shiftKey || ev.ctrlKey; // Check if the focused task's context menu is open - if so, skip arrow navigation shortcuts @@ -193,13 +213,6 @@ export class TaskShortcutService { return true; } - // Toggle play/pause - if (checkKeyCombo(ev, keys.togglePlay) && this.isTimeTrackingEnabled()) { - this._handleTaskShortcut(focusedTaskId, 'togglePlayPause'); - ev.preventDefault(); - return true; - } - // Task movement shortcuts if (checkKeyCombo(ev, keys.moveTaskUp)) { this._handleTaskShortcut(focusedTaskId, 'moveTaskUp');