Skip to content

Commit 18eefa9

Browse files
guitavanoclaude
andauthored
fix(sections-editor): avoid double breadcrumb prefix in module-loader unions + add tests (#4482)
Addresses PR review findings: - Blocker: the SchemaForm consumed-prefix re-prepend double-counted for module-loader-union fields, because AnyOfField ALSO re-prepends its own crumb. It stripped its crumb conditionally (only when at the head) but re-prepended unconditionally. Make the re-prepend symmetric with the strip so it composes correctly with the parent's re-prepend — no more ["Página de Listagem", "Página de Listagem", …] when drilling into a loader config. - Testing: add Playwright CT regressions for the two focus-loss scenarios (array-label == item-label alt-driven label; editing a label to collide with an earlier sibling keeps the opened row). NOTE: the array drill-down CT tests do not execute in this local environment (they fail identically on origin/main here) and CT is not part of CI — added as the correct-layer regression guard following the existing pattern. - Docs: note the consumedBreadcrumbPrefix <-> breadcrumbPathForActiveField tail-slice coupling, and the ArrayField reconciliation fixed-point + list-only mutation invariants that keep openIndex from desyncing. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 21bd33a commit 18eefa9

4 files changed

Lines changed: 111 additions & 6 deletions

File tree

apps/mesh/ct/tests/array-object-drilldown.ct.tsx

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,87 @@ test("editing a drilled-in item updates the correct index", async ({
160160
.toEqual({ cards: [{ name: "Alpha" }, { name: "Beta2" }] });
161161
});
162162

163+
test("editing a label field whose value equals the array label keeps the editor open", async ({
164+
mount,
165+
}) => {
166+
// Regression: an array labelled "Banner" whose single item is ALSO labelled
167+
// "Banner" (its label comes from `alt` via titleBy). The breadcrumb is
168+
// ["Banner", "Banner"]; editing `alt` must not collapse the trail and kick
169+
// you back to the list. Guards the SchemaForm consumed-prefix re-prepend.
170+
const meta = sectionWithProps({
171+
banner: {
172+
type: "array",
173+
title: "Banner",
174+
items: {
175+
type: "object",
176+
title: "Banner",
177+
titleBy: "alt",
178+
properties: { alt: { type: "string", title: "Alt" } },
179+
},
180+
},
181+
});
182+
const component = await mount(
183+
<SchemaFormHarness
184+
meta={meta}
185+
resolveType={TEST_RESOLVE_TYPE}
186+
initialValue={{ banner: [{ alt: "Banner" }] }}
187+
/>,
188+
);
189+
190+
await component.getByRole("button", { name: "Banner", exact: true }).click();
191+
await expect
192+
.poll(() => readBreadcrumb(component))
193+
.toEqual(["Banner", "Banner"]);
194+
195+
const altInput = component.getByLabel("Alt");
196+
await expect(altInput).toHaveAttribute("id", "banner.0.alt");
197+
await altInput.fill("Banner Sale");
198+
199+
// Editor stays open on the same item (does NOT drop back to the list), and
200+
// the edit is applied.
201+
await expect(component.getByLabel("Alt")).toHaveAttribute(
202+
"id",
203+
"banner.0.alt",
204+
);
205+
await expect(component.getByRole("button", { name: "Add item" })).toHaveCount(
206+
0,
207+
);
208+
await expect
209+
.poll(() => readFormValue(component))
210+
.toEqual({ banner: [{ alt: "Banner Sale" }] });
211+
});
212+
213+
test("editing a label to collide with an earlier sibling keeps the opened row", async ({
214+
mount,
215+
}) => {
216+
// Regression: drill into item 1, rename it to equal item 0's label. Selection
217+
// must stay on index 1 (not snap to the first matching label at index 0).
218+
// Guards the ArrayField openIndex/preferredIndex disambiguation.
219+
const meta = sectionWithProps(cardArrayProps);
220+
const component = await mount(
221+
<SchemaFormHarness
222+
meta={meta}
223+
resolveType={TEST_RESOLVE_TYPE}
224+
initialValue={{ cards: [{ name: "Alpha" }, { name: "Beta" }] }}
225+
/>,
226+
);
227+
228+
await component.getByRole("button", { name: "Beta", exact: true }).click();
229+
const nameInput = component.getByLabel("Name");
230+
await expect(nameInput).toHaveAttribute("id", "cards.1.name");
231+
232+
await nameInput.fill("Alpha");
233+
234+
// Still editing index 1 — not snapped to the identically-labelled index 0.
235+
await expect(component.getByLabel("Name")).toHaveAttribute(
236+
"id",
237+
"cards.1.name",
238+
);
239+
await expect
240+
.poll(() => readFormValue(component))
241+
.toEqual({ cards: [{ name: "Alpha" }, { name: "Alpha" }] });
242+
});
243+
163244
test("deleting a row removes the right item from the array", async ({
164245
mount,
165246
page,

apps/mesh/src/web/components/sections-editor/fields/any-of-field.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,19 @@ export function AnyOfField({
178178
// both label and key, so resolution still works.
179179
const outerCrumb = label ?? (path.includes(".") ? path.split(".")[0]! : path);
180180
const safeBreadcrumbPath = breadcrumbPath ?? [];
181-
const nestedBreadcrumbPath =
182-
isModuleLoaderUnion && safeBreadcrumbPath[0] === outerCrumb
183-
? safeBreadcrumbPath.slice(1)
184-
: safeBreadcrumbPath;
181+
// Strip our own crumb only when it's actually at the head. The parent
182+
// SchemaForm may have already consumed it (via breadcrumbPathForActiveField)
183+
// and now re-prepends it for us (via consumedBreadcrumbPrefix). Re-prepend
184+
// SYMMETRICALLY — add the crumb back only when we stripped it here; otherwise
185+
// the parent's re-prepend would double it (["Página de Listagem", "Página de
186+
// Listagem", …]) when this loader union is the active narrowed field.
187+
const strippedOwnCrumb =
188+
isModuleLoaderUnion && safeBreadcrumbPath[0] === outerCrumb;
189+
const nestedBreadcrumbPath = strippedOwnCrumb
190+
? safeBreadcrumbPath.slice(1)
191+
: safeBreadcrumbPath;
185192
const nestedOnBreadcrumbChange: ((p: string[]) => void) | undefined =
186-
isModuleLoaderUnion
193+
strippedOwnCrumb
187194
? (newPath) => {
188195
onBreadcrumbChange?.([outerCrumb, ...newPath]);
189196
}

apps/mesh/src/web/components/sections-editor/fields/array-field.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,15 @@ export function ArrayField({
342342
// Keep the tracked index in step with the breadcrumb: forget it once the
343343
// breadcrumb no longer opens an item here, and adopt the resolved index when
344344
// navigation opened an item some other way (header/breadcrumb click, deep link).
345+
//
346+
// This set-state-during-render reconciliation converges in one extra render
347+
// because `resolveArrayItemSelection` returns `preferredIndex` only when that
348+
// item still matches the winning crumb: adopting the resolved index yields a
349+
// fixed point on the next render (no oscillation). `openIndex` is a raw index
350+
// (not tied to the stable `entries` ids), which is safe only because every
351+
// index-shifting mutation (reorder, duplicate, remove) is reachable exclusively
352+
// from the list view below — where `selection` is null and `openIndex` has
353+
// already been forced back to null.
345354
if (selection === null) {
346355
if (openIndex !== null) setOpenIndex(null);
347356
} else if (selection.index !== openIndex) {

apps/mesh/src/web/components/sections-editor/schema-form-breadcrumb.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,15 @@ export function consumedBreadcrumbPrefix(
6969
);
7070
}
7171

72-
/** Drop crumbs consumed by the active field so children see a relative trail. */
72+
/**
73+
* Drop crumbs consumed by the active field so children see a relative trail.
74+
*
75+
* NOTE: {@link consumedBreadcrumbPrefix} reconstructs the dropped prefix purely
76+
* by length difference, which relies on the return value always being a
77+
* front-suffix of `breadcrumbPath` (this only ever drops the leading crumb).
78+
* Keep it that way — dropping from the middle or rewriting a crumb would make
79+
* that reconstruction silently wrong.
80+
*/
7381
export function breadcrumbPathForActiveField(
7482
activeKey: string,
7583
schema: SchemaProperty,

0 commit comments

Comments
 (0)