From 3ae9d968a4dcc77d0500a407251c0dff11d4513b Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Mon, 29 Jun 2026 15:57:44 +0200 Subject: [PATCH] fix(tasks): make "Add subtask" work in the Planner detail panel (#8617) (#8630) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 row that renders the parent. The Planner renders tasks as , so the request was dropped and nothing happened (regression from #8423; Today view still worked). - task-detail-panel now hosts its own inline (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 . - 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 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 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. --- .../planner-add-subtask-from-detail.spec.ts | 57 ++++++++++++++ ...add-subtask-with-detail-panel-open.spec.ts | 55 +++++++++++++ .../add-subtask-input.service.ts | 8 +- .../task-detail-item.component.html | 6 +- .../task-detail-item.component.ts | 6 ++ .../task-detail-panel.component.html | 16 ++-- .../task-detail-panel.component.spec.ts | 41 +++++++--- .../task-detail-panel.component.ts | 77 +++++++++++++++++-- 8 files changed, 240 insertions(+), 26 deletions(-) create mode 100644 e2e/tests/planner/planner-add-subtask-from-detail.spec.ts diff --git a/e2e/tests/planner/planner-add-subtask-from-detail.spec.ts b/e2e/tests/planner/planner-add-subtask-from-detail.spec.ts new file mode 100644 index 0000000000..5272e565bc --- /dev/null +++ b/e2e/tests/planner/planner-add-subtask-from-detail.spec.ts @@ -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 , not . The detail panel's + * "Add subtask" used to delegate (via a shared signal) to the 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', + ); + }); +}); diff --git a/e2e/tests/task-list-basic/add-subtask-with-detail-panel-open.spec.ts b/e2e/tests/task-list-basic/add-subtask-with-detail-panel-open.spec.ts index 3a65b43385..64bd93ee7d 100644 --- a/e2e/tests/task-list-basic/add-subtask-with-detail-panel-open.spec.ts +++ b/e2e/tests/task-list-basic/add-subtask-with-detail-panel-open.spec.ts @@ -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 => { + 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', + ); + }); }); diff --git a/src/app/features/tasks/add-subtask-input/add-subtask-input.service.ts b/src/app/features/tasks/add-subtask-input/add-subtask-input.service.ts index 151174b1ba..be1bae41d4 100644 --- a/src/app/features/tasks/add-subtask-input/add-subtask-input.service.ts +++ b/src/app/features/tasks/add-subtask-input/add-subtask-input.service.ts @@ -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 `` + * 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 `` 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, diff --git a/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.html b/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.html index f48b320ac1..fe50b5deb2 100644 --- a/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.html +++ b/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.html @@ -30,7 +30,11 @@ } @if (type === 'panel') { - + diff --git a/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.ts b/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.ts index 2390f97d21..6f80fbf11e 100644 --- a/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.ts +++ b/src/app/features/tasks/task-detail-panel/task-additional-info-item/task-detail-item.component.ts @@ -49,6 +49,12 @@ export class TaskDetailItemComponent { readonly collapseParent = output(); readonly keyPress = output(); + // Emits when the expansion state changes (header click, keyboard, or + // programmatic), so a parent can keep a controlling signal in sync. + readonly expandedChange = output(); + // 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(); readonly editActionTriggered = output(); @HostBinding('tabindex') readonly tabindex: number = 3; diff --git a/src/app/features/tasks/task-detail-panel/task-detail-panel.component.html b/src/app/features/tasks/task-detail-panel/task-detail-panel.component.html index d6eddef10c..ab966b820f 100644 --- a/src/app/features/tasks/task-detail-panel/task-detail-panel.component.html +++ b/src/app/features/tasks/task-detail-panel/task-detail-panel.component.html @@ -15,11 +15,9 @@ @@ -45,8 +43,16 @@ } } + @if (isAddSubtaskInputVisible()) { + + }