mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-29 10:40:12 +00:00
* fix(tasks): make detail-panel add-subtask work in the Planner (#8617) The detail panel and context menu delegated "Add subtask" to AddSubtaskInputService.requestOpen(), a signal consumed only by the <task> row that renders the parent. The Planner renders tasks as <planner-task>, so the request was dropped and nothing happened (regression from #8423; Today view still worked). - task-detail-panel now hosts its own inline <add-subtask-input> (works in every view; also fixes the input opening behind the bottom panel on mobile). It controls the sub-task section's expansion via a signal and focuses the input after the expand animation completes; animates in/out with [@expandFade]. - task-detail-item gains expandedChange/afterExpand outputs so the panel can control/observe the Material expansion panel. - context menu addSubTask() reverts to direct addSubTaskTo() (a transient menu has no place to host the inline draft). Adds a Planner e2e repro and updates the affected unit specs. * fix(tasks): address multi-review findings for #8617 add-subtask - Focus the inline input via a deferred timeout in onSubTasksAfterExpand: with animations disabled Material fires afterExpand synchronously within the same CD pass, before the addSubtaskInput viewChild is committed, so the first collapsed→expand "Add subtask" click left the input unfocused. - Reset isSubTasksExpanded on task switch so the sub-task section doesn't stay sticky-expanded across tasks (the panel instance is reused). - Return focus to the "Add subtask" button when the draft is closed via Escape, instead of dropping focus to <body>. - Refresh the now-stale AddSubtaskInputService doc comment. - Assert the draft input is focused in the Planner e2e (the focus path was previously uncovered). * fix(tasks): keep context-menu add-subtask on the inline-draft bus The earlier context-menu change to addSubTaskTo() was unnecessary: the context menu's "Add sub-task" entry is gated behind isAdvancedControls, which only the <task> row enables. planner-task and schedule-event leave it false, so the entry is hidden there — meaning the menu action is only ever reachable from a rendered <task> row, where requestOpen() works. Reverting restores the v18.12 inline-draft UX for that path and shrinks the diff. (#8617 was only ever reachable via the detail panel, which the self-hosted input fix already covers.) * test(tasks): cover add-subtask focus with animations disabled Guards the deferred-focus fix: with animations disabled Material fires the expansion panel's afterExpand synchronously, before the panel's add-subtask-input viewChild is committed. Verified this test fails without the setTimeout deferral and passes with it.
This commit is contained in:
parent
cb13912fad
commit
3ae9d968a4
8 changed files with 240 additions and 26 deletions
57
e2e/tests/planner/planner-add-subtask-from-detail.spec.ts
Normal file
57
e2e/tests/planner/planner-add-subtask-from-detail.spec.ts
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
import { test, expect } from '../../fixtures/test.fixture';
|
||||
import { PlannerPage } from '../../pages/planner.page';
|
||||
import { cssSelectors } from '../../constants/selectors';
|
||||
|
||||
const { DETAIL_PANEL } = cssSelectors;
|
||||
|
||||
/**
|
||||
* Repro for #8617: "Add subtask is not working in the task detail panel" when
|
||||
* the panel is opened from the Planner.
|
||||
*
|
||||
* The Planner renders tasks as <planner-task>, not <task>. The detail panel's
|
||||
* "Add subtask" used to delegate (via a shared signal) to the <task> row that
|
||||
* renders the parent, which does not exist in the Planner — so nothing happened.
|
||||
*/
|
||||
test.describe('Planner: add subtask from detail panel', () => {
|
||||
test('adds a subtask via the detail panel opened from a planner task', async ({
|
||||
page,
|
||||
workViewPage,
|
||||
}) => {
|
||||
const plannerPage = new PlannerPage(page);
|
||||
await workViewPage.waitForTaskList();
|
||||
await workViewPage.addTask('Planner Parent Task');
|
||||
|
||||
await plannerPage.navigateToPlanner();
|
||||
await plannerPage.waitForPlannerView();
|
||||
|
||||
const plannerTask = page
|
||||
.locator('planner-task')
|
||||
.filter({ hasText: 'Planner Parent Task' });
|
||||
await expect(plannerTask).toBeVisible({ timeout: 15000 });
|
||||
|
||||
// Clicking the planner task selects it -> the detail panel opens.
|
||||
await plannerTask.click();
|
||||
const panel = page.locator(DETAIL_PANEL);
|
||||
await expect(panel).toBeVisible({ timeout: 5000 });
|
||||
|
||||
// The sub-task section starts collapsed; expand it to reveal the button.
|
||||
await panel
|
||||
.locator('mat-expansion-panel-header')
|
||||
.filter({ hasText: 'Subtasks' })
|
||||
.click();
|
||||
|
||||
await panel.getByRole('button', { name: 'Add subtask' }).click();
|
||||
|
||||
// The inline draft input must open inside the panel (the bug: it never did)
|
||||
// and be focused so the user can type straight away.
|
||||
const input = page.locator('.e2e-add-subtask-input');
|
||||
await expect(input).toBeVisible({ timeout: 3000 });
|
||||
await expect(input).toBeFocused();
|
||||
await input.fill('Sub from planner');
|
||||
await page.keyboard.press('Enter');
|
||||
|
||||
await expect(panel.locator('.sub-tasks task task-title')).toContainText(
|
||||
'Sub from planner',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -19,6 +19,29 @@ const { DETAIL_PANEL } = cssSelectors;
|
|||
test.describe('Add subtask with detail panel open', () => {
|
||||
const draftInput = (page: Page): Locator => page.locator('.e2e-add-subtask-input');
|
||||
|
||||
const disableAnimations = async (page: Page): Promise<void> => {
|
||||
await expect
|
||||
.poll(() =>
|
||||
page.evaluate(() => {
|
||||
const store = (
|
||||
window as unknown as {
|
||||
__e2eTestHelpers?: { store?: { dispatch: (a: unknown) => void } };
|
||||
}
|
||||
).__e2eTestHelpers?.store;
|
||||
if (!store) return false;
|
||||
store.dispatch({
|
||||
type: '[Global Config] Update Global Config Section',
|
||||
sectionKey: 'misc',
|
||||
sectionCfg: { isDisableAnimations: true },
|
||||
isSkipSnack: true,
|
||||
});
|
||||
return true;
|
||||
}),
|
||||
)
|
||||
.toBe(true);
|
||||
await expect(page.locator('body.isDisableAnimations')).toBeVisible();
|
||||
};
|
||||
|
||||
const addParentAndOpenPanel = async (
|
||||
page: Page,
|
||||
workViewPage: WorkViewPage,
|
||||
|
|
@ -129,4 +152,36 @@ test.describe('Add subtask with detail panel open', () => {
|
|||
|
||||
await expect(parent.locator('.sub-tasks task')).toHaveCount(2);
|
||||
});
|
||||
|
||||
// With animations disabled, Material fires the expansion panel's afterExpand
|
||||
// synchronously (inside the same change-detection pass), before the panel's
|
||||
// add-subtask-input viewChild is committed. The focus must still land — it is
|
||||
// deferred a tick precisely for this case. Regression guard for that fix.
|
||||
test('opens and focuses the inline draft when animations are disabled', async ({
|
||||
page,
|
||||
workViewPage,
|
||||
taskPage,
|
||||
}) => {
|
||||
await disableAnimations(page);
|
||||
const parent = await addParentAndOpenPanel(page, workViewPage, taskPage);
|
||||
|
||||
await expect
|
||||
.poll(async () =>
|
||||
page.evaluate(() => !!document.activeElement?.closest('task-detail-panel')),
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
// Section starts collapsed, so this goes through the afterExpand focus path.
|
||||
await page.keyboard.press('a');
|
||||
|
||||
const input = draftInput(page);
|
||||
await expect(input).toBeVisible();
|
||||
await expect(input).toBeFocused();
|
||||
await input.fill('SubTask no anim');
|
||||
await page.keyboard.press('Enter');
|
||||
|
||||
await expect(parent.locator('.sub-tasks task task-title').first()).toContainText(
|
||||
'SubTask no anim',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,9 +2,11 @@ import { Injectable, signal } from '@angular/core';
|
|||
|
||||
/**
|
||||
* Bus that tells the `TaskComponent` rendering `parentId`'s subtasks to open its
|
||||
* inline draft input. Callers (row, context menu, shortcut, detail panel) don't
|
||||
* hold a reference to that component, so the request travels through this shared
|
||||
* signal.
|
||||
* inline draft input. The in-list row, its `taskAddSubTask` shortcut, and the
|
||||
* task context menu (whose "Add sub-task" entry only shows on rendered `<task>`
|
||||
* rows) don't hold a reference to that component, so the request travels through
|
||||
* this shared signal. (The detail panel hosts its own input instead, so it works
|
||||
* in views like the Planner where no `<task>` row renders the parent — see #8617.)
|
||||
*
|
||||
* The request is transient: the consuming row calls `consume()` once it has
|
||||
* acted on it, resetting the signal to `null`. This is deliberate — without it,
|
||||
|
|
|
|||
|
|
@ -30,7 +30,11 @@
|
|||
</section>
|
||||
}
|
||||
@if (type === 'panel') {
|
||||
<mat-expansion-panel [expanded]="expanded">
|
||||
<mat-expansion-panel
|
||||
[expanded]="expanded"
|
||||
(expandedChange)="expanded = $event; expandedChange.emit($event)"
|
||||
(afterExpand)="afterExpand.emit()"
|
||||
>
|
||||
<mat-expansion-panel-header (click)="focusEl()">
|
||||
<mat-panel-title>
|
||||
<ng-content select="[panel-header]"></ng-content>
|
||||
|
|
|
|||
|
|
@ -49,6 +49,12 @@ export class TaskDetailItemComponent {
|
|||
|
||||
readonly collapseParent = output<void>();
|
||||
readonly keyPress = output<KeyboardEvent>();
|
||||
// Emits when the expansion state changes (header click, keyboard, or
|
||||
// programmatic), so a parent can keep a controlling signal in sync.
|
||||
readonly expandedChange = output<boolean>();
|
||||
// Emits once the expand animation has finished, so a parent can focus content
|
||||
// that is only focusable after the panel body becomes visible.
|
||||
readonly afterExpand = output<void>();
|
||||
readonly editActionTriggered = output<void>();
|
||||
|
||||
@HostBinding('tabindex') readonly tabindex: number = 3;
|
||||
|
|
|
|||
|
|
@ -15,11 +15,9 @@
|
|||
<task-detail-item
|
||||
(collapseParent)="collapseParent()"
|
||||
(keyPress)="onItemKeyPress($event)"
|
||||
[expanded]="
|
||||
isSubTaskPanelExpandedInitially() &&
|
||||
(task().subTasks?.length || 0) > 0 &&
|
||||
!isDialogMode()
|
||||
"
|
||||
[expanded]="isSubTasksExpanded()"
|
||||
(expandedChange)="isSubTasksExpanded.set($event)"
|
||||
(afterExpand)="onSubTasksAfterExpand()"
|
||||
[type]="'panel'"
|
||||
>
|
||||
<ng-container panel-header>
|
||||
|
|
@ -45,8 +43,16 @@
|
|||
}
|
||||
</div>
|
||||
}
|
||||
@if (isAddSubtaskInputVisible()) {
|
||||
<add-subtask-input
|
||||
[@expandFade]
|
||||
[parentId]="task().id"
|
||||
(closed)="onAddSubtaskInputClosed($event)"
|
||||
></add-subtask-input>
|
||||
}
|
||||
<div class="add-btn-wrapper">
|
||||
<button
|
||||
#addSubTaskBtn
|
||||
(click)="addSubTask()"
|
||||
mat-button
|
||||
>
|
||||
|
|
|
|||
|
|
@ -18,7 +18,6 @@ import { TranslateModule, TranslateService } from '@ngx-translate/core';
|
|||
import { MarkdownModule } from 'ngx-markdown';
|
||||
import { DEFAULT_TASK, TaskWithSubTasks } from '../task.model';
|
||||
import { TaskDetailItemComponent } from './task-additional-info-item/task-detail-item.component';
|
||||
import { AddSubtaskInputService } from '../add-subtask-input/add-subtask-input.service';
|
||||
|
||||
const MOCK_TASK: TaskWithSubTasks = {
|
||||
...(DEFAULT_TASK as TaskWithSubTasks),
|
||||
|
|
@ -339,7 +338,6 @@ describe('TaskDetailPanelComponent stale-focus guard', () => {
|
|||
describe('TaskDetailPanelComponent add sub-task', () => {
|
||||
let component: TaskDetailPanelComponent;
|
||||
let addSubTaskToSpy: jasmine.Spy;
|
||||
let addSubtaskInputServiceSpy: jasmine.SpyObj<AddSubtaskInputService>;
|
||||
|
||||
const keydown = (key: string, target: HTMLElement): KeyboardEvent => {
|
||||
const ev = new KeyboardEvent('keydown', {
|
||||
|
|
@ -353,10 +351,6 @@ describe('TaskDetailPanelComponent add sub-task', () => {
|
|||
|
||||
beforeEach(async () => {
|
||||
addSubTaskToSpy = jasmine.createSpy('addSubTaskTo').and.returnValue('new-sub-id');
|
||||
addSubtaskInputServiceSpy = jasmine.createSpyObj<AddSubtaskInputService>(
|
||||
'AddSubtaskInputService',
|
||||
['requestOpen'],
|
||||
);
|
||||
|
||||
await TestBed.configureTestingModule({
|
||||
imports: [TaskDetailPanelComponent],
|
||||
|
|
@ -371,10 +365,10 @@ describe('TaskDetailPanelComponent add sub-task', () => {
|
|||
setSelectedId: () => undefined,
|
||||
focusTaskIfPossible: () => undefined,
|
||||
addSubTaskTo: addSubTaskToSpy,
|
||||
showSubTasks: () => undefined,
|
||||
focusTaskById: () => undefined,
|
||||
},
|
||||
},
|
||||
{ provide: AddSubtaskInputService, useValue: addSubtaskInputServiceSpy },
|
||||
{ provide: TaskAttachmentService, useValue: {} },
|
||||
{ provide: ClipboardImageService, useValue: {} },
|
||||
{ provide: LayoutService, useValue: {} },
|
||||
|
|
@ -405,12 +399,41 @@ describe('TaskDetailPanelComponent add sub-task', () => {
|
|||
fixture.detectChanges();
|
||||
});
|
||||
|
||||
it('requests the inline subtask input for the shown task', () => {
|
||||
it('shows the inline subtask input in the panel for a top-level task', () => {
|
||||
component.addSubTask();
|
||||
expect(addSubtaskInputServiceSpy.requestOpen).toHaveBeenCalledWith('P');
|
||||
// The input is hosted by the panel itself (not delegated to a <task> row),
|
||||
// so it works regardless of which view opened the panel (#8617).
|
||||
expect(component.isAddSubtaskInputVisible()).toBe(true);
|
||||
expect(component.isSubTasksExpanded()).toBe(true);
|
||||
expect(addSubTaskToSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('creates a sibling directly when adding a subtask from a subtask panel', () => {
|
||||
const fixture = TestBed.createComponent(TaskDetailPanelComponent);
|
||||
const subComponent = fixture.componentInstance;
|
||||
fixture.componentRef.setInput('task', {
|
||||
id: 'SUB',
|
||||
parentId: 'P',
|
||||
subTasks: [],
|
||||
tagIds: [],
|
||||
} as unknown as TaskWithSubTasks);
|
||||
fixture.detectChanges();
|
||||
|
||||
subComponent.addSubTask();
|
||||
|
||||
expect(addSubTaskToSpy).toHaveBeenCalledWith('P');
|
||||
expect(subComponent.isAddSubtaskInputVisible()).toBe(false);
|
||||
});
|
||||
|
||||
it('hides the input again when it is closed', () => {
|
||||
component.addSubTask();
|
||||
expect(component.isAddSubtaskInputVisible()).toBe(true);
|
||||
component.onAddSubtaskInputClosed('blur');
|
||||
expect(component.isAddSubtaskInputVisible()).toBe(false);
|
||||
// The section stays expanded so the just-added subtasks remain visible.
|
||||
expect(component.isSubTasksExpanded()).toBe(true);
|
||||
});
|
||||
|
||||
it('routes the add-subtask shortcut to addSubTask (with prevent/stopPropagation)', () => {
|
||||
spyOn(component, 'addSubTask');
|
||||
const ev = keydown('a', document.createElement('div'));
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import {
|
|||
Component,
|
||||
computed,
|
||||
DestroyRef,
|
||||
ElementRef,
|
||||
HostListener,
|
||||
inject,
|
||||
input,
|
||||
|
|
@ -30,6 +31,7 @@ import { T } from '../../../t.const';
|
|||
import { TaskService } from '../task.service';
|
||||
import {
|
||||
expandAnimation,
|
||||
expandFadeAnimation,
|
||||
expandFadeInOnlyAnimation,
|
||||
} from '../../../ui/animations/expand.ani';
|
||||
import { fadeAnimation } from '../../../ui/animations/fade.ani';
|
||||
|
|
@ -87,14 +89,23 @@ import { checkKeyCombo } from '../../../util/check-key-combo';
|
|||
import { IS_MAC } from '../../../util/is-mac';
|
||||
import { ClipboardImageService } from '../../../core/clipboard-image/clipboard-image.service';
|
||||
import { DropPasteIcons } from '../../../core/drop-paste-input/drop-paste.model';
|
||||
import { AddSubtaskInputService } from '../add-subtask-input/add-subtask-input.service';
|
||||
import {
|
||||
AddSubtaskInputComponent,
|
||||
AddSubtaskInputCloseReason,
|
||||
} from '../add-subtask-input/add-subtask-input.component';
|
||||
|
||||
@Component({
|
||||
selector: 'task-detail-panel',
|
||||
templateUrl: './task-detail-panel.component.html',
|
||||
styleUrls: ['./task-detail-panel.component.scss'],
|
||||
changeDetection: ChangeDetectionStrategy.OnPush,
|
||||
animations: [expandAnimation, expandFadeInOnlyAnimation, fadeAnimation, swirlAnimation],
|
||||
animations: [
|
||||
expandAnimation,
|
||||
expandFadeAnimation,
|
||||
expandFadeInOnlyAnimation,
|
||||
fadeAnimation,
|
||||
swirlAnimation,
|
||||
],
|
||||
imports: [
|
||||
TaskTitleComponent,
|
||||
TaskDetailItemComponent,
|
||||
|
|
@ -115,6 +126,7 @@ import { AddSubtaskInputService } from '../add-subtask-input/add-subtask-input.s
|
|||
MsToStringPipe,
|
||||
TranslatePipe,
|
||||
IssueIconPipe,
|
||||
AddSubtaskInputComponent,
|
||||
],
|
||||
})
|
||||
export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
|
|
@ -132,7 +144,6 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro
|
|||
private _translateService = inject(TranslateService);
|
||||
private _destroyRef = inject(DestroyRef);
|
||||
private _dateTimeFormatService = inject(DateTimeFormatService);
|
||||
private _addSubtaskInputService = inject(AddSubtaskInputService);
|
||||
|
||||
// Inputs
|
||||
task = input.required<TaskWithSubTasks>();
|
||||
|
|
@ -143,6 +154,16 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro
|
|||
itemEls = viewChildren(TaskDetailItemComponent);
|
||||
attachmentPanelElRef = viewChild<TaskDetailItemComponent>('attachmentPanelElRef');
|
||||
noteWrapperElRef = viewChild<TaskDetailItemComponent>('noteWrapperElRef');
|
||||
addSubtaskInput = viewChild(AddSubtaskInputComponent);
|
||||
addSubTaskBtn = viewChild<ElementRef<HTMLButtonElement>>('addSubTaskBtn');
|
||||
|
||||
// The detail panel hosts its own inline subtask draft input rather than
|
||||
// delegating to the <task> row that renders the parent: in the Planner (and
|
||||
// other non-list views) that row does not exist, so the delegated request was
|
||||
// silently dropped (#8617). isSubTasksExpanded controls the sub-task section
|
||||
// so the input is visible even when triggered while the section is collapsed.
|
||||
readonly isAddSubtaskInputVisible = signal(false);
|
||||
readonly isSubTasksExpanded = signal(false);
|
||||
|
||||
// Constants
|
||||
IS_TOUCH_PRIMARY = IS_TOUCH_PRIMARY;
|
||||
|
|
@ -352,10 +373,6 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro
|
|||
return task && !task.parentId;
|
||||
});
|
||||
|
||||
isSubTaskPanelExpandedInitially = computed(() => {
|
||||
return this.isDialogMode();
|
||||
});
|
||||
|
||||
showTimeEstimate = computed(() => !this.task().subTasks?.length);
|
||||
|
||||
hasAttachments = computed(() => {
|
||||
|
|
@ -442,6 +459,11 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro
|
|||
takeUntilDestroyed(this._destroyRef),
|
||||
)
|
||||
.subscribe(() => {
|
||||
// Don't carry a half-open subtask draft or the expanded sub-task section
|
||||
// over to the next task (the panel component is reused across tasks,
|
||||
// unlike per-row <task> components).
|
||||
this.isAddSubtaskInputVisible.set(false);
|
||||
this.isSubTasksExpanded.set(false);
|
||||
// Only auto-focus panel content when focus is already inside the panel,
|
||||
// to avoid stealing focus from the main task list during navigation (#6578)
|
||||
if (document.activeElement?.closest('task-detail-panel')) {
|
||||
|
|
@ -615,7 +637,46 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro
|
|||
|
||||
addSubTask(): void {
|
||||
const task = this.task();
|
||||
this._addSubtaskInputService.requestOpen(task.parentId || task.id);
|
||||
// The sub-task section (and thus the inline input) only renders for a
|
||||
// top-level task. On a subtask's own panel "add subtask" means "add a
|
||||
// sibling under my parent" — there is no section to host the input, so
|
||||
// create it directly (matches the pre-inline-draft behaviour).
|
||||
if (task.parentId) {
|
||||
this.taskService.addSubTaskTo(task.parentId);
|
||||
return;
|
||||
}
|
||||
|
||||
if (task._hideSubTasksMode === HideSubTasksMode.HideAll) {
|
||||
this.taskService.showSubTasks(task.id);
|
||||
}
|
||||
const wasExpanded = this.isSubTasksExpanded();
|
||||
this.isSubTasksExpanded.set(true);
|
||||
this.isAddSubtaskInputVisible.set(true);
|
||||
// When already expanded the input is immediately focusable. When we just
|
||||
// expanded it, the panel body is visibility:hidden until the expand
|
||||
// animation finishes — onSubTasksAfterExpand() handles focus in that case.
|
||||
if (wasExpanded) {
|
||||
window.setTimeout(() => this.addSubtaskInput()?.focus());
|
||||
}
|
||||
}
|
||||
|
||||
onSubTasksAfterExpand(): void {
|
||||
// Defer focus: with animations disabled Material fires afterExpand
|
||||
// synchronously inside the same change-detection pass, before the
|
||||
// addSubtaskInput viewChild is committed (it would be undefined here).
|
||||
if (this.isAddSubtaskInputVisible()) {
|
||||
window.setTimeout(() => this.addSubtaskInput()?.focus());
|
||||
}
|
||||
}
|
||||
|
||||
onAddSubtaskInputClosed(reason: AddSubtaskInputCloseReason): void {
|
||||
this.isAddSubtaskInputVisible.set(false);
|
||||
// Keep the sub-task section expanded so the just-added sub-tasks stay
|
||||
// visible. On Escape (a keyboard cancel) return focus to the trigger so
|
||||
// keyboard navigation continues from the panel rather than falling to body.
|
||||
if (reason === 'escape') {
|
||||
window.setTimeout(() => this.addSubTaskBtn()?.nativeElement.focus());
|
||||
}
|
||||
}
|
||||
|
||||
collapseParent(): void {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue