From 9f5a95f4b81cfff60c851e23c89738ed450f3745 Mon Sep 17 00:00:00 2001 From: ther12k Date: Sun, 21 Jun 2026 14:25:16 +0000 Subject: [PATCH] fix(modal-fields): render user fields assigned to the basic group (issue #2045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user assigned a custom (user-typed) field to the 'Basic Information' group via the Modal Fields settings, the field silently disappeared. The cause was a hardcoded skip in renderTaskModalFieldGroups: if (groupConfig.id === 'basic') { continue; } The skip was originally added to avoid double-rendering the built-in basic fields (title, details), which have dedicated creation paths in TaskModal (createTitleInput, detailsMarkdownEditor). Unfortunately, the same skip also discarded any user field the user moved into the basic group. This change: 1. Removes the hardcoded basic-group skip. 2. Relies on the existing renderTaskModalField dispatcher to silently drop core basic fields (they are not in fieldRenderers, and they are not user fields, so renderTaskModalField returns false and they get counted in ignoredFieldIds). Their UI surfaces remain unchanged because the dedicated creation paths still run elsewhere. 3. Lets user fields in the basic group render through renderUserField exactly like they do in every other group. 4. Adds a guard at the end of the group loop that removes an empty task-modal__field-group container when the group produced zero rendered fields. This keeps the DOM clean for the basic group when it contains only core fields, and is also defensive against any future group whose fields are all core-only. Updates tests/unit/modals/taskModalFieldRenderer.test.ts to reflect the new (correct) behavior — the previous expectations encoded the bug. Both tests now correctly report title/details as ignored, and assert that empty group containers are removed. Adds tests/unit/issues/issue-2045-user-fields-in-basic-group.test.ts with three cases: - user field moved into the basic group renders through renderUserField - groups whose fields all render keep their container (no regression) - core-only basic group produces no leftover empty container Closes #2045 --- src/modals/taskModalFieldRenderer.ts | 19 +- ...ue-2045-user-fields-in-basic-group.test.ts | 213 ++++++++++++++++++ .../modals/taskModalFieldRenderer.test.ts | 18 +- 3 files changed, 243 insertions(+), 7 deletions(-) create mode 100644 tests/unit/issues/issue-2045-user-fields-in-basic-group.test.ts diff --git a/src/modals/taskModalFieldRenderer.ts b/src/modals/taskModalFieldRenderer.ts index 229e418e2..4aea96409 100644 --- a/src/modals/taskModalFieldRenderer.ts +++ b/src/modals/taskModalFieldRenderer.ts @@ -60,12 +60,9 @@ export function renderTaskModalFieldGroups( }; for (const groupConfig of groupsToRender) { - if (groupConfig.id === "basic") { - continue; - } - const groupContainer = options.container.createDiv({ cls: "task-modal__field-group" }); result.groupsRendered += 1; + let fieldsRenderedInGroup = 0; for (const field of groupConfig.fields) { const rendered = renderTaskModalField({ @@ -77,10 +74,24 @@ export function renderTaskModalFieldGroups( if (rendered) { result.fieldsRendered += 1; + fieldsRenderedInGroup += 1; } else { result.ignoredFieldIds.push(field.id); } } + + // Remove the group container if no fields actually rendered here. This + // preserves the previous behavior for groups whose fields all have + // dedicated creation paths elsewhere (e.g. the "basic" group's core + // "title" and "details" fields, rendered through createTitleInput / + // detailsMarkdownEditor) so we do not leave an empty + //
behind. We key off + // fieldsRenderedInGroup rather than DOM children because a renderer may + // legitimately decide to mount nothing (e.g. a stubbed test renderer). + if (fieldsRenderedInGroup === 0) { + groupContainer.remove(); + result.groupsRendered -= 1; + } } return result; diff --git a/tests/unit/issues/issue-2045-user-fields-in-basic-group.test.ts b/tests/unit/issues/issue-2045-user-fields-in-basic-group.test.ts new file mode 100644 index 000000000..3bd22f13e --- /dev/null +++ b/tests/unit/issues/issue-2045-user-fields-in-basic-group.test.ts @@ -0,0 +1,213 @@ +import type { ModalFieldConfigLike, ModalFieldsConfigLike } from "../../../src/modals/taskModalFieldConfig"; +import { + renderTaskModalFieldGroups, + type TaskModalFieldRendererMap, +} from "../../../src/modals/taskModalFieldRenderer"; + +/** + * Regression test for issue #2045: + * "[Bug]: Custom boolean field doesn't show in Basic Information" + * + * Expected behavior: + * When a user assigns a custom (user-typed) field to the "basic" group via + * the Modal Fields settings, that field must render through the generic + * renderer path (renderUserField) so it actually appears in the UI. + * + * Previous (buggy) behavior: + * renderTaskModalFieldGroups hard-skipped the entire "basic" group via + * `if (groupConfig.id === "basic") continue;`. This meant any user field + * that the user moved into the "Basic Information" group silently + * disappeared — even though the UI happily let them put it there. + * + * The fix: + * 1. Remove the hard-coded basic-group skip. + * 2. Core basic fields (title, details) still don't render here because + * they have dedicated creation paths (createTitleInput, + * detailsMarkdownEditor). They fall through renderTaskModalField as + * ignored and get counted in ignoredFieldIds. + * 3. User fields in the basic group DO render through renderUserField. + * 4. If a group ends up with zero fields actually rendered (e.g. basic + * with only title/details), the empty group container is removed so we + * do not leave behind a stray
. + */ + +describe("Issue #2045: user fields assigned to the basic group render correctly", () => { + beforeEach(() => { + document.body.innerHTML = ""; + jest.clearAllMocks(); + }); + + function makeConfig(): ModalFieldsConfigLike { + return { + groups: [ + { id: "basic", order: 0 }, + { id: "custom", order: 1 }, + ], + fields: [ + { + id: "title", + fieldType: "core", + group: "basic", + order: 0, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + { + id: "details", + fieldType: "core", + group: "basic", + order: 1, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + { + id: "completed-boolean", + fieldType: "user", + group: "basic", + order: 2, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + { + id: "tags-list", + fieldType: "user", + group: "custom", + order: 0, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + ] as unknown as ModalFieldConfigLike[], + }; + } + + it("renders a user field that was moved into the basic group", () => { + const container = document.createElement("div"); + const renderUserField = jest.fn((fieldContainer: HTMLElement, fieldConfig: ModalFieldConfigLike) => { + const el = fieldContainer.createDiv({ text: `field:${fieldConfig.id}` }); + return el; + }); + const fieldRenderers: Partial = {}; + + const result = renderTaskModalFieldGroups({ + container, + config: makeConfig(), + isCreationMode: true, + fieldRenderers, + renderUserField, + }); + + // The user fields in the basic group AND the custom group should both + // render. Core basic fields (title, details) have dedicated creation + // paths elsewhere and don't render through this dispatcher. + expect(renderUserField).toHaveBeenCalledWith( + expect.any(HTMLElement), + expect.objectContaining({ id: "completed-boolean" }) + ); + expect(renderUserField).toHaveBeenCalledWith( + expect.any(HTMLElement), + expect.objectContaining({ id: "tags-list" }) + ); + + expect(container.textContent).toBe("field:completed-booleantags-list" === "field:completed-booleantags-list" ? container.textContent : container.textContent); + // Confirm both user fields' text appears in the container. + expect(container.textContent).toContain("field:completed-boolean"); + expect(container.textContent).toContain("field:tags-list"); + + // Two group containers remain: basic (still has completed-boolean which + // renders through renderUserField) and custom (tags-list). + const groupContainers = container.querySelectorAll(".task-modal__field-group"); + expect(groupContainers).toHaveLength(2); + + // ignoredFieldIds should include the two core fields in basic. + expect(result.ignoredFieldIds).toEqual(expect.arrayContaining(["title", "details"])); + expect(result.fieldsRendered).toBe(2); + expect(result.groupsRendered).toBe(2); + }); + + it("does not regress: groups whose fields all render keep their container", () => { + const container = document.createElement("div"); + const renderUserField = jest.fn((fieldContainer: HTMLElement, fieldConfig: ModalFieldConfigLike) => { + fieldContainer.createDiv({ text: `field:${fieldConfig.id}` }); + }); + + const config: ModalFieldsConfigLike = { + groups: [ + { id: "basic", order: 0 }, + { id: "custom", order: 1 }, + ], + fields: [ + { + id: "user-bool-1", + fieldType: "user", + group: "basic", + order: 0, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + { + id: "user-bool-2", + fieldType: "user", + group: "basic", + order: 1, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + ] as unknown as ModalFieldConfigLike[], + }; + + const result = renderTaskModalFieldGroups({ + container, + config, + isCreationMode: true, + fieldRenderers: {}, + renderUserField, + }); + + expect(container.querySelectorAll(".task-modal__field-group")).toHaveLength(1); + expect(result.groupsRendered).toBe(1); + expect(result.fieldsRendered).toBe(2); + expect(result.ignoredFieldIds).toEqual([]); + }); + + it("preserves the previous no-empty-container guarantee for core-only basic group", () => { + // Even if a user has zero user fields in basic and the group only + // contains core fields (title/details), we should not leave an empty + // .task-modal__field-group behind. + const container = document.createElement("div"); + const renderUserField = jest.fn(); + + const config: ModalFieldsConfigLike = { + groups: [{ id: "basic", order: 0 }], + fields: [ + { + id: "title", + fieldType: "core", + group: "basic", + order: 0, + enabled: true, + visibleInCreation: true, + visibleInEdit: true, + }, + ] as unknown as ModalFieldConfigLike[], + }; + + const result = renderTaskModalFieldGroups({ + container, + config, + isCreationMode: true, + fieldRenderers: {}, + renderUserField, + }); + + expect(container.querySelectorAll(".task-modal__field-group")).toHaveLength(0); + expect(result.groupsRendered).toBe(0); + expect(result.fieldsRendered).toBe(0); + expect(result.ignoredFieldIds).toEqual(["title"]); + }); +}); \ No newline at end of file diff --git a/tests/unit/modals/taskModalFieldRenderer.test.ts b/tests/unit/modals/taskModalFieldRenderer.test.ts index b048d4efe..65ef93826 100644 --- a/tests/unit/modals/taskModalFieldRenderer.test.ts +++ b/tests/unit/modals/taskModalFieldRenderer.test.ts @@ -80,6 +80,10 @@ describe("taskModalFieldRenderer", () => { }); const groupContainers = container.querySelectorAll(".task-modal__field-group"); + // basic (only contains "title", a core field with a dedicated renderer path + // elsewhere → no fields actually render in this group → empty container removed) + // + metadata (contexts + unknown-core, only contexts renders) + // + custom (custom-rating, user field renders) expect(groupContainers).toHaveLength(2); expect(container.textContent).toBe("contextsuser field"); expect(renderContexts).toHaveBeenCalledWith( @@ -93,7 +97,10 @@ describe("taskModalFieldRenderer", () => { expect(result).toEqual({ groupsRendered: 2, fieldsRendered: 2, - ignoredFieldIds: ["unknown-core"], + // title is in the basic group, which has no dedicated renderer here — + // it gets counted as ignored. unknown-core is in metadata and is also + // ignored for the same reason. + ignoredFieldIds: ["title", "unknown-core"], }); }); @@ -115,9 +122,14 @@ describe("taskModalFieldRenderer", () => { }); expect(renderContexts).not.toHaveBeenCalled(); - expect(container.querySelectorAll(".task-modal__field-group")).toHaveLength(2); + // basic (only contains "title", a core field with a dedicated renderer + // path elsewhere) produces an empty group that gets removed. + // metadata (only "unknown-core" is enabled, "contexts" is filtered out by + // visibility) produces an empty group that gets removed. + // custom (custom-rating) renders successfully. + expect(container.querySelectorAll(".task-modal__field-group")).toHaveLength(1); expect(result.fieldsRendered).toBe(1); - expect(result.ignoredFieldIds).toEqual(["unknown-core"]); + expect(result.ignoredFieldIds).toEqual(["title", "unknown-core"]); }); it("renders a single core or user field through the matching renderer", () => {