From fb61ed7d925c2adaaf0c93257d2061afe0669b34 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Mon, 27 Apr 2026 13:49:12 +0200 Subject: [PATCH] fix(boards): re-enable Save in Eisenhower Matrix edit dialog (#7380) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conditionally-required match-mode and sortDir radios placed `defaultValue` inside `props`, which Formly ignores — its core extension reads `field.defaultValue`. So opening any board with >=2 included or excluded tags left those required fields undefined and the Save button disabled. Even with the default lifted, Formly skips defaults for fields hidden at init and on hidden→visible transitions unless `resetOnHide: true` is set, so picking a sortBy re-locked the form. Lift `defaultValue` to field level and add `resetOnHide: true` on `includedTagsMatch`, `excludedTagsMatch`, and `sortDir`. Add a unit spec exercising the panel fieldGroup and a Playwright e2e that drives the dialog end-to-end. --- .../boards/issue-7380-eisenhower-edit.spec.ts | 71 +++++++++ .../features/boards/boards-form.const.spec.ts | 138 ++++++++++++++++++ src/app/features/boards/boards-form.const.ts | 17 ++- 3 files changed, 222 insertions(+), 4 deletions(-) create mode 100644 e2e/tests/boards/issue-7380-eisenhower-edit.spec.ts create mode 100644 src/app/features/boards/boards-form.const.spec.ts diff --git a/e2e/tests/boards/issue-7380-eisenhower-edit.spec.ts b/e2e/tests/boards/issue-7380-eisenhower-edit.spec.ts new file mode 100644 index 0000000000..107161f0fe --- /dev/null +++ b/e2e/tests/boards/issue-7380-eisenhower-edit.spec.ts @@ -0,0 +1,71 @@ +import { expect, test } from '../../fixtures/test.fixture'; + +/** + * Regression for #7380: opening the edit dialog for the default Eisenhower + * Matrix board left the Save button disabled, because Formly's required + * `includedTagsMatch` / `excludedTagsMatch` radios never received their + * `defaultValue` (it was placed inside `props`, which Formly ignores). + */ +test.describe('Boards — Eisenhower Matrix edit dialog (#7380)', () => { + test('Save is enabled on open and after picking a sortBy', async ({ + page, + workViewPage, + }) => { + await workViewPage.waitForTaskList(); + + await page.goto('/#/boards'); + await page.waitForLoadState('networkidle'); + await expect(page.locator('boards')).toBeVisible({ timeout: 10000 }); + + // The Eisenhower Matrix board ships with `cols: 2` and four panels. + // Find the Eisenhower tab and select it. + const eisenhowerTab = page + .locator('mat-tab-group [role="tab"]') + .filter({ hasText: /eisenhower/i }) + .first(); + await eisenhowerTab.click(); + + // Default Eisenhower Matrix references Important / Urgent tags that + // don't exist in a fresh profile — click "Create Tags" if shown so the + // panels actually render. + const createTagsBtn = page.locator('button', { hasText: /create tags?/i }); + if (await createTagsBtn.isVisible().catch(() => false)) { + await createTagsBtn.click(); + } + + // Open the edit dialog via the per-panel edit (pencil) button — the only + // way to edit a board on a desktop viewport without right-click. + const firstPanelEditBtn = page + .locator('board-panel header button[mat-icon-button]') + .first(); + await expect(firstPanelEditBtn).toBeVisible({ timeout: 10000 }); + await firstPanelEditBtn.click(); + + const dialog = page.locator('dialog-board-edit'); + await expect(dialog).toBeVisible({ timeout: 5000 }); + + const saveBtn = dialog.locator('button:has(mat-icon:has-text("save"))'); + await expect(saveBtn).toBeVisible(); + + // Pre-fix: this expectation fails — the form is invalid on open because + // `includedTagsMatch` for URGENT_AND_IMPORTANT is required-but-undefined. + await expect(saveBtn).toBeEnabled(); + + // Now exercise the second part of the bug: pick a sort column on the + // first panel; sortDir radio is revealed with `defaultValue: 'asc'`. + // Drilling into Formly internals via DOM is fragile, so we just confirm + // the form stays enabled after toggling the first sort-by select to its + // first non-manual option. + const sortBySelect = dialog.locator('mat-select').first(); + await sortBySelect.click(); + // Pick the first option that isn't the currently-selected "manual" entry. + const sortOption = page.locator('mat-option').nth(1); + await sortOption.click(); + + await expect(saveBtn).toBeEnabled(); + + // Click Save and the dialog should close cleanly. + await saveBtn.click(); + await expect(dialog).toBeHidden({ timeout: 5000 }); + }); +}); diff --git a/src/app/features/boards/boards-form.const.spec.ts b/src/app/features/boards/boards-form.const.spec.ts new file mode 100644 index 0000000000..c85aa3505c --- /dev/null +++ b/src/app/features/boards/boards-form.const.spec.ts @@ -0,0 +1,138 @@ +import { Component } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { UntypedFormGroup } from '@angular/forms'; +import { + FieldType, + FormlyFieldConfig, + FormlyFormBuilder, + FormlyModule, +} from '@ngx-formly/core'; +import { BOARDS_FORM } from './boards-form.const'; +import { DEFAULT_BOARDS, DEFAULT_PANEL_CFG } from './boards.const'; +import { BoardPanelCfg } from './boards.model'; + +@Component({ selector: 'noop-formly-field', template: '', standalone: true }) +class NoopFormlyFieldComponent extends FieldType {} + +/** + * Regression for #7380. Editing a default Eisenhower Matrix board left the + * Save button disabled. Two compounding issues: + * + * 1. `defaultValue` was set inside `props` for the conditionally-required + * radios, but Formly only reads `field.defaultValue`. So the model never + * got `'all'`/`'any'` and `required: true` fails the form on open for + * any panel with >=2 included or excluded tags. + * 2. Even with `defaultValue` lifted to field level, Formly skips the + * default for fields hidden at init AND when they later become visible + * (only `resetOnHide: true` triggers the late-apply path). + * + * The test exercises the inner panel fieldGroup directly so we don't need + * the custom `repeat`/material widgets registered. + */ +describe('BOARDS_FORM panel behavior (#7380)', () => { + const TYPE_STUBS = [ + 'input', + 'select', + 'radio', + 'checkbox', + 'tag-select', + 'project-select', + 'repeat', + ].map((name) => ({ name, component: NoopFormlyFieldComponent })); + + let builder: FormlyFormBuilder; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [NoopFormlyFieldComponent, FormlyModule.forRoot({ types: TYPE_STUBS })], + }); + builder = TestBed.inject(FormlyFormBuilder); + }); + + const panelFieldGroup = (): FormlyFieldConfig[] => { + const panels = BOARDS_FORM.find((f) => f.key === 'panels')!; + const fieldArray = panels.fieldArray as FormlyFieldConfig; + // Deep clone so each test gets isolated field state. + return JSON.parse(JSON.stringify(fieldArray.fieldGroup)); + }; + + const eisenhowerPanel = (idx: number): BoardPanelCfg => { + const src = DEFAULT_BOARDS.find((b) => b.id === 'EISENHOWER_MATRIX')!; + return { ...DEFAULT_PANEL_CFG, ...src.panels[idx] }; + }; + + const buildPanelForm = ( + panel: BoardPanelCfg, + ): { form: UntypedFormGroup; fields: FormlyFieldConfig[]; model: BoardPanelCfg } => { + const fields = panelFieldGroup(); + const form = new UntypedFormGroup({}); + const model = JSON.parse(JSON.stringify(panel)); + builder.buildForm(form, fields, model, {}); + return { form, fields, model }; + }; + + const fieldByKey = ( + fields: FormlyFieldConfig[], + key: keyof BoardPanelCfg, + ): FormlyFieldConfig => fields.find((f) => f.key === key)!; + + it('starts valid for the URGENT_AND_IMPORTANT panel (2 included tags)', () => { + const { form, model } = buildPanelForm(eisenhowerPanel(0)); + expect(form.valid).toBe(true); + // includedTagsMatch is visible at init (length 2) so its defaultValue + // ('all') is applied normally. + expect(model.includedTagsMatch).toBe('all'); + }); + + it('keeps the form valid after picking a sortBy (sortDir gets its default)', () => { + const { form, fields, model } = buildPanelForm(eisenhowerPanel(0)); + expect(form.valid).toBe(true); + + const sortBy = fieldByKey(fields, 'sortBy'); + sortBy.formControl!.setValue('dueDate'); + model.sortBy = 'dueDate'; + + // Re-evaluate hide expressions — what `(modelChange)` triggers in the + // running dialog. `checkExpressions` walks the tree and flips hide. + const opts = sortBy.options as any; + opts.checkExpressions({ fieldGroup: fields, options: opts }); + + expect(model.sortDir).toBe('asc'); + expect(form.valid).toBe(true); + }); + + it('clears sortDir and stays valid when sortBy reverts to manual', () => { + const { form, fields, model } = buildPanelForm(eisenhowerPanel(0)); + const sortBy = fieldByKey(fields, 'sortBy'); + const opts = sortBy.options as any; + + sortBy.formControl!.setValue('dueDate'); + model.sortBy = 'dueDate'; + opts.checkExpressions({ fieldGroup: fields, options: opts }); + expect(model.sortDir).toBe('asc'); + + sortBy.formControl!.setValue(null); + model.sortBy = undefined; + opts.checkExpressions({ fieldGroup: fields, options: opts }); + + expect(model.sortDir).toBeUndefined(); + expect(form.valid).toBe(true); + }); + + it('reveals includedTagsMatch with its default when a 2nd tag is added', () => { + // Single-tag panel: includedTagsMatch is hidden at init. + const { form, fields, model } = buildPanelForm(eisenhowerPanel(2)); + expect(form.valid).toBe(true); + expect(model.includedTagsMatch).toBeUndefined(); + + const includedTagIds = fieldByKey(fields, 'includedTagIds'); + const opts = includedTagIds.options as any; + const next = [...(model.includedTagIds ?? []), 'extra-tag']; + includedTagIds.formControl!.setValue(next); + model.includedTagIds = next; + opts.checkExpressions({ fieldGroup: fields, options: opts }); + + expect(model.includedTagsMatch).toBe('all'); + expect(form.valid).toBe(true); + }); +}); diff --git a/src/app/features/boards/boards-form.const.ts b/src/app/features/boards/boards-form.const.ts index 560842bce6..25a7158997 100644 --- a/src/app/features/boards/boards-form.const.ts +++ b/src/app/features/boards/boards-form.const.ts @@ -68,14 +68,21 @@ export const BOARDS_FORM: LimitedFormlyFieldConfig[] = [ { key: 'includedTagsMatch', type: 'radio', - // Only meaningful with >=2 required tags + // Only meaningful with >=2 required tags. expressions: { hide: 'model.includedTagIds?.length < 2', }, + // `defaultValue` lives at the field level — Formly's core extension + // reads `field.defaultValue`, not `field.props.defaultValue`, when + // populating the model. `resetOnHide` makes the default flow into + // the model when the field transitions hidden→visible. Without + // these, the field is shown with `undefined` and the `required` + // validator locks the Save button (#7380). + defaultValue: 'all', + resetOnHide: true, props: { label: T.F.BOARDS.FORM.TAGS_MATCH_MODE, required: true, - defaultValue: 'all', options: [ { value: 'all', label: T.F.BOARDS.FORM.TAGS_MATCH_ALL }, { value: 'any', label: T.F.BOARDS.FORM.TAGS_MATCH_ANY }, @@ -99,10 +106,11 @@ export const BOARDS_FORM: LimitedFormlyFieldConfig[] = [ expressions: { hide: 'model.excludedTagIds?.length < 2', }, + defaultValue: 'any', + resetOnHide: true, props: { label: T.F.BOARDS.FORM.TAGS_EXCLUDED_MATCH_MODE, required: true, - defaultValue: 'any', options: [ { value: 'any', label: T.F.BOARDS.FORM.TAGS_EXCLUDED_MATCH_ANY }, { value: 'all', label: T.F.BOARDS.FORM.TAGS_EXCLUDED_MATCH_ALL }, @@ -180,10 +188,11 @@ export const BOARDS_FORM: LimitedFormlyFieldConfig[] = [ expressions: { hide: '!model.sortBy', }, + defaultValue: 'asc', + resetOnHide: true, props: { label: T.F.BOARDS.FORM.SORT_DIR, required: true, - defaultValue: 'asc', options: [ { value: 'asc', label: T.F.BOARDS.FORM.SORT_DIR_ASC }, { value: 'desc', label: T.F.BOARDS.FORM.SORT_DIR_DESC },