diff --git a/electron/shared-with-frontend/keyboard-config.model.ts b/electron/shared-with-frontend/keyboard-config.model.ts index 0eeb356b4c..8e830c570e 100644 --- a/electron/shared-with-frontend/keyboard-config.model.ts +++ b/electron/shared-with-frontend/keyboard-config.model.ts @@ -59,6 +59,14 @@ export type KeyboardConfig = Readonly<{ [key: `plugin_${string}`]: string | null; }>; +/** Shared frozen empty keyboard config so the "no shortcuts" path returns a + * referentially-stable value instead of allocating a fresh {} every CD pass. */ +export const EMPTY_KEYBOARD_CONFIG: KeyboardConfig = Object.freeze({}) as KeyboardConfig; + +export const keyboardConfigOrEmpty = ( + keyboard: KeyboardConfig | undefined, +): KeyboardConfig => keyboard ?? EMPTY_KEYBOARD_CONFIG; + export const GLOBAL_KEY_CFG_KEYS: (keyof KeyboardConfig)[] = [ 'globalShowHide', 'globalToggleTaskStart', diff --git a/src/app/core-ui/main-header/main-header.component.ts b/src/app/core-ui/main-header/main-header.component.ts index 4cdaf4e611..0410f34aaa 100644 --- a/src/app/core-ui/main-header/main-header.component.ts +++ b/src/app/core-ui/main-header/main-header.component.ts @@ -23,7 +23,7 @@ import { SyncWrapperService } from '../../imex/sync/sync-wrapper.service'; import { SnackService } from '../../core/snack/snack.service'; import { NavigationEnd, Router } from '@angular/router'; import { GlobalConfigService } from '../../features/config/global-config.service'; -import { KeyboardConfig } from '@sp/keyboard-config'; +import { KeyboardConfig, keyboardConfigOrEmpty } from '@sp/keyboard-config'; import { MatIconButton } from '@angular/material/button'; import { MatIcon } from '@angular/material/icon'; import { MatTooltip } from '@angular/material/tooltip'; @@ -342,6 +342,6 @@ export class MainHeaderComponent implements OnDestroy { } get kb(): KeyboardConfig { - return (this._configService.cfg()?.keyboard as KeyboardConfig) || {}; + return keyboardConfigOrEmpty(this._configService.cfg()?.keyboard as KeyboardConfig); } } diff --git a/src/app/features/config/keyboard-config.const.spec.ts b/src/app/features/config/keyboard-config.const.spec.ts new file mode 100644 index 0000000000..7e85b1c56e --- /dev/null +++ b/src/app/features/config/keyboard-config.const.spec.ts @@ -0,0 +1,24 @@ +import { + EMPTY_KEYBOARD_CONFIG, + KeyboardConfig, + keyboardConfigOrEmpty, +} from '@sp/keyboard-config'; + +describe('keyboardConfigOrEmpty', () => { + it('returns EMPTY_KEYBOARD_CONFIG for undefined', () => { + expect(keyboardConfigOrEmpty(undefined)).toBe(EMPTY_KEYBOARD_CONFIG); + }); + + it('returns a referentially-stable empty config across calls', () => { + expect(keyboardConfigOrEmpty(undefined)).toBe(keyboardConfigOrEmpty(undefined)); + }); + + it('returns the same reference for a provided config', () => { + const someKb: KeyboardConfig = { addNewTask: 'n' }; + expect(keyboardConfigOrEmpty(someKb)).toBe(someKb); + }); + + it('EMPTY_KEYBOARD_CONFIG is frozen', () => { + expect(Object.isFrozen(EMPTY_KEYBOARD_CONFIG)).toBe(true); + }); +}); diff --git a/src/app/features/planner/planner-task/planner-task.component.html b/src/app/features/planner/planner-task/planner-task.component.html index 8aa6b1453e..2dfce50856 100644 --- a/src/app/features/planner/planner-task/planner-task.component.html +++ b/src/app/features/planner/planner-task/planner-task.component.html @@ -1,7 +1,7 @@
{{ parentTitle }}
} - @if (titleHasLinks) { + @if (titleHasLinks()) {
} @else { -
{{ task.title }}
+
{{ task().title }}
}
- @if (task.subTaskIds?.length) { + @if (task().subTaskIds?.length) { - account_tree{{ task.subTaskIds.length }} + account_tree{{ task().subTaskIds?.length }} }
@@ -65,6 +65,6 @@ @if (isContextMenuLoaded()) { } diff --git a/src/app/features/planner/planner-task/planner-task.component.spec.ts b/src/app/features/planner/planner-task/planner-task.component.spec.ts new file mode 100644 index 0000000000..94113a244c --- /dev/null +++ b/src/app/features/planner/planner-task/planner-task.component.spec.ts @@ -0,0 +1,126 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { NO_ERRORS_SCHEMA, signal, WritableSignal } from '@angular/core'; +import { TranslateModule } from '@ngx-translate/core'; +import { of } from 'rxjs'; +import { PlannerTaskComponent } from './planner-task.component'; +import { TaskService } from '../../tasks/task.service'; +import { DEFAULT_TASK, TaskCopy } from '../../tasks/task.model'; +import { MsToStringPipe } from '../../../ui/duration/ms-to-string.pipe'; +import { RenderLinksPipe } from '../../../ui/pipes/render-links.pipe'; +import { TranslatePipe } from '@ngx-translate/core'; + +const makeTask = (overrides: Partial = {}): TaskCopy => + ({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 't1', + parentId: null, + ...overrides, + }) as TaskCopy; + +describe('PlannerTaskComponent', () => { + let currentTaskId: WritableSignal; + + const create = ( + task: TaskCopy, + ): { + fixture: ComponentFixture; + component: PlannerTaskComponent; + } => { + const fixture = TestBed.createComponent(PlannerTaskComponent); + fixture.componentRef.setInput('task', task); + fixture.detectChanges(); + return { fixture, component: fixture.componentInstance }; + }; + + beforeEach(() => { + currentTaskId = signal(null); + const taskServiceMock = { + ...jasmine.createSpyObj('TaskService', [ + 'setSelectedId', + 'toggleDoneWithAnimation', + 'update', + ]), + currentTaskId, + getByIdLive$: () => of(null), + }; + + TestBed.configureTestingModule({ + imports: [PlannerTaskComponent, TranslateModule.forRoot()], + schemas: [NO_ERRORS_SCHEMA], + providers: [{ provide: TaskService, useValue: taskServiceMock }], + }); + + // Isolate the component from its heavyweight child components (tag-list etc.) + // so the spec exercises PlannerTaskComponent's OWN template bindings without + // needing the full Store/service graph. Child elements become unknown tags + // (ignored via NO_ERRORS_SCHEMA); the pipes used in the template are kept. + TestBed.overrideComponent(PlannerTaskComponent, { + set: { + imports: [MsToStringPipe, RenderLinksPipe, TranslatePipe], + schemas: [NO_ERRORS_SCHEMA], + }, + }); + }); + + it('renders the template without throwing', () => { + expect(() => create(makeTask({ title: 'plain title' }))).not.toThrow(); + }); + + describe('titleHasLinks', () => { + it('is false for a plain title', () => { + const { component } = create(makeTask({ title: 'plain title' })); + expect(component.titleHasLinks()).toBe(false); + }); + + it('is true for a title containing a URL', () => { + const { component } = create(makeTask({ title: 'see https://example.com' })); + expect(component.titleHasLinks()).toBe(true); + }); + }); + + describe('timeEstimate', () => { + it('returns the raw estimate when the task has subTaskIds', () => { + const { component } = create( + makeTask({ timeEstimate: 5000, timeSpent: 2000, subTaskIds: ['s1'] }), + ); + expect(component.timeEstimate()).toBe(5000); + }); + }); + + describe('isCurrent', () => { + it('is true when the current task id equals the task id and reflects on the host', () => { + currentTaskId.set('t1'); + const { fixture, component } = create(makeTask({ id: 't1' })); + expect(component.isCurrent()).toBe(true); + expect(fixture.debugElement.nativeElement.classList.contains('isCurrent')).toBe( + true, + ); + }); + + it('flips to false when the current task id changes', () => { + currentTaskId.set('t1'); + const { fixture, component } = create(makeTask({ id: 't1' })); + expect(component.isCurrent()).toBe(true); + + currentTaskId.set('other'); + fixture.detectChanges(); + expect(component.isCurrent()).toBe(false); + expect(fixture.debugElement.nativeElement.classList.contains('isCurrent')).toBe( + false, + ); + }); + }); + + describe('isDone host class', () => { + it('carries the isDone class when the task is done', () => { + const { fixture } = create(makeTask({ isDone: true })); + expect(fixture.debugElement.nativeElement.classList.contains('isDone')).toBe(true); + }); + + it('omits the isDone class when the task is not done', () => { + const { fixture } = create(makeTask({ isDone: false })); + expect(fixture.debugElement.nativeElement.classList.contains('isDone')).toBe(false); + }); + }); +}); diff --git a/src/app/features/planner/planner-task/planner-task.component.ts b/src/app/features/planner/planner-task/planner-task.component.ts index 62db56338e..685191763d 100644 --- a/src/app/features/planner/planner-task/planner-task.component.ts +++ b/src/app/features/planner/planner-task/planner-task.component.ts @@ -3,12 +3,11 @@ import { ChangeDetectionStrategy, ChangeDetectorRef, Component, + computed, DestroyRef, ElementRef, - HostBinding, HostListener, inject, - Input, input, OnDestroy, OnInit, @@ -49,21 +48,26 @@ import { TranslatePipe } from '@ngx-translate/core'; SwipeBlockComponent, TranslatePipe, ], + /* eslint-disable @typescript-eslint/naming-convention */ + host: { + '[class.isDone]': 'task().isDone', + '[class.isDragReady]': 'isDragReady()', + '[class.isCurrent]': 'isCurrent()', + }, + /* eslint-enable @typescript-eslint/naming-convention */ }) export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { private _taskService = inject(TaskService); private _cd = inject(ChangeDetectorRef); private _destroyRef = inject(DestroyRef); private _elementRef = inject(ElementRef); - get titleHasLinks(): boolean { - const title = this.task?.title; - return !!title && hasLinkHints(title); - } - // TODO: Skipped for migration because: - // This input is used in a control flow expression (e.g. `@if` or `*ngIf`) - // and migrating would break narrowing currently. - @Input({ required: true }) task!: TaskCopy; + readonly task = input.required(); + + readonly titleHasLinks = computed(() => { + const title = this.task().title; + return !!title && hasLinkHints(title); + }); // TODO remove readonly day = input(); @@ -84,20 +88,9 @@ export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { read: TaskContextMenuComponent, }); - @HostBinding('class.isDone') - get isDone(): boolean { - return this.task.isDone; - } - - @HostBinding('class.isDragReady') - get isDragReadyClass(): boolean { - return this.isDragReady(); - } - - @HostBinding('class.isCurrent') - get isCurrent(): boolean { - return this.task.id === this._taskService.currentTaskId(); - } + readonly isCurrent = computed( + () => this.task().id === this._taskService.currentTaskId(), + ); @HostListener('contextmenu', ['$event']) onContextMenu(event: MouseEvent): void { @@ -114,25 +107,24 @@ export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { if (target?.tagName === 'A' || target?.closest('a')) { return; } - if (this.task) { - // Use bottom panel on mobile, dialog on desktop - this._taskService.setSelectedId(this.task.id); - } + // Use bottom panel on mobile, dialog on desktop + this._taskService.setSelectedId(this.task().id); } - get timeEstimate(): number { - const t = this.task; - return this.task.subTaskIds + readonly timeEstimate = computed(() => { + const t = this.task(); + return t.subTaskIds ? t.timeEstimate : t.timeEstimate - t.timeSpent > 0 ? t.timeEstimate - t.timeSpent : 0; - } + }); ngOnInit(): void { - if (this.task.parentId) { + const parentId = this.task().parentId; + if (parentId) { this._taskService - .getByIdLive$(this.task.parentId) + .getByIdLive$(parentId) .pipe(takeUntilDestroyed(this._destroyRef)) .subscribe((parentTask) => { this.parentTitle = parentTask && parentTask.title; @@ -179,7 +171,7 @@ export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { } onSwipeRightTriggered(isTriggered: boolean): void { - if (this.task.isDone) { + if (this.task().isDone) { this.showUndoneAnimation.set(isTriggered); } else { this.showDoneAnimation.set(isTriggered); @@ -188,9 +180,10 @@ export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { toggleTaskDone(): void { window.clearTimeout(this._doneAnimationTimeout); + const t = this.task(); this._doneAnimationTimeout = this._taskService.toggleDoneWithAnimation( - this.task.id, - this.task.isDone, + t.id, + t.isDone, (v) => this.showDoneAnimation.set(v), ); } @@ -216,7 +209,7 @@ export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { } updateTimeEstimate(val: number): void { - this._taskService.update(this.task.id, { + this._taskService.update(this.task().id, { timeEstimate: val, }); } diff --git a/src/app/features/tasks/task/task.component.ts b/src/app/features/tasks/task/task.component.ts index cc1f6a3bfa..f2f0d0f159 100644 --- a/src/app/features/tasks/task/task.component.ts +++ b/src/app/features/tasks/task/task.component.ts @@ -71,7 +71,11 @@ import { DateService } from '../../../core/date/date.service'; import { isTouchActive } from '../../../util/input-intent'; import { IS_HYBRID_DEVICE } from '../../../util/is-mouse-primary'; import { DRAG_DELAY_FOR_TOUCH } from '../../../app.constants'; -import { KeyboardConfig } from '@sp/keyboard-config'; +import { + EMPTY_KEYBOARD_CONFIG, + KeyboardConfig, + keyboardConfigOrEmpty, +} from '@sp/keyboard-config'; import { DialogScheduleTaskComponent } from '../../planner/dialog-schedule-task/dialog-schedule-task.component'; import { PlannerActions } from '../../planner/store/planner.actions'; import { PlannerService } from '../../planner/planner.service'; @@ -1394,9 +1398,9 @@ export class TaskComponent implements OnDestroy, AfterViewInit { get kb(): KeyboardConfig { if (isTouchActive()) { - return {} as KeyboardConfig; + return EMPTY_KEYBOARD_CONFIG; } - return (this._configService.cfg()?.keyboard as KeyboardConfig) || {}; + return keyboardConfigOrEmpty(this._configService.cfg()?.keyboard as KeyboardConfig); } protected readonly ICAL_TYPE = ICAL_TYPE;