mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-01-23 02:36:05 +00:00
fix(schedule): start tracking selected task when pressing Y in schedule view
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
This commit is contained in:
parent
2af57d2b4a
commit
acedc67f2a
2 changed files with 305 additions and 23 deletions
269
src/app/features/tasks/task-shortcut.service.spec.ts
Normal file
269
src/app/features/tasks/task-shortcut.service.spec.ts
Normal file
|
|
@ -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<typeof signal<string | null>>;
|
||||||
|
lastFocusedTaskComponent: ReturnType<typeof signal<any>>;
|
||||||
|
};
|
||||||
|
let mockTaskService: jasmine.SpyObj<TaskService> & {
|
||||||
|
selectedTaskId: ReturnType<typeof signal<string | null>>;
|
||||||
|
currentTaskId: ReturnType<typeof signal<string | null>>;
|
||||||
|
};
|
||||||
|
let mockConfigService: {
|
||||||
|
cfg: ReturnType<typeof signal<any>>;
|
||||||
|
};
|
||||||
|
|
||||||
|
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<string | null>(null),
|
||||||
|
lastFocusedTaskComponent: signal<any>(null),
|
||||||
|
};
|
||||||
|
|
||||||
|
mockTaskService = {
|
||||||
|
selectedTaskId: signal<string | null>(null),
|
||||||
|
currentTaskId: signal<string | null>(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);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
import { computed, inject, Injectable } from '@angular/core';
|
import { computed, inject, Injectable } from '@angular/core';
|
||||||
import { TaskFocusService } from './task-focus.service';
|
import { TaskFocusService } from './task-focus.service';
|
||||||
|
import { TaskService } from './task.service';
|
||||||
import { GlobalConfigService } from '../config/global-config.service';
|
import { GlobalConfigService } from '../config/global-config.service';
|
||||||
import { checkKeyCombo } from '../../util/check-key-combo';
|
import { checkKeyCombo } from '../../util/check-key-combo';
|
||||||
import { Log } from '../../core/log';
|
import { Log } from '../../core/log';
|
||||||
|
|
@ -35,6 +36,7 @@ type TaskComponentMethod = keyof TaskComponent;
|
||||||
})
|
})
|
||||||
export class TaskShortcutService {
|
export class TaskShortcutService {
|
||||||
private readonly _taskFocusService = inject(TaskFocusService);
|
private readonly _taskFocusService = inject(TaskFocusService);
|
||||||
|
private readonly _taskService = inject(TaskService);
|
||||||
private readonly _configService = inject(GlobalConfigService);
|
private readonly _configService = inject(GlobalConfigService);
|
||||||
readonly isTimeTrackingEnabled = computed(
|
readonly isTimeTrackingEnabled = computed(
|
||||||
() => this._configService.cfg()?.appFeatures.isTimeTrackingEnabled,
|
() => this._configService.cfg()?.appFeatures.isTimeTrackingEnabled,
|
||||||
|
|
@ -47,26 +49,44 @@ export class TaskShortcutService {
|
||||||
* @returns True if the shortcut was handled, false otherwise
|
* @returns True if the shortcut was handled, false otherwise
|
||||||
*/
|
*/
|
||||||
handleTaskShortcuts(ev: KeyboardEvent): boolean {
|
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();
|
const cfg = this._configService.cfg();
|
||||||
if (!cfg) return false;
|
if (!cfg) return false;
|
||||||
|
|
||||||
const keys = cfg.keyboard;
|
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;
|
const isShiftOrCtrlPressed = ev.shiftKey || ev.ctrlKey;
|
||||||
|
|
||||||
// Check if the focused task's context menu is open - if so, skip arrow navigation shortcuts
|
// 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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Toggle play/pause
|
|
||||||
if (checkKeyCombo(ev, keys.togglePlay) && this.isTimeTrackingEnabled()) {
|
|
||||||
this._handleTaskShortcut(focusedTaskId, 'togglePlayPause');
|
|
||||||
ev.preventDefault();
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Task movement shortcuts
|
// Task movement shortcuts
|
||||||
if (checkKeyCombo(ev, keys.moveTaskUp)) {
|
if (checkKeyCombo(ev, keys.moveTaskUp)) {
|
||||||
this._handleTaskShortcut(focusedTaskId, 'moveTaskUp');
|
this._handleTaskShortcut(focusedTaskId, 'moveTaskUp');
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue