Frontend: Fix People discard dialog races and stacked Add-name prompt #4966

This commit is contained in:
Michael Mayer 2026-05-15 15:37:51 +00:00
parent a37882418f
commit 005c34dd5e
2 changed files with 123 additions and 10 deletions

View file

@ -1385,10 +1385,11 @@ export default {
onAddNameConfirm() {
const { markerUid, name } = this.addNameDialog;
this.addNameDialog = { visible: false, markerUid: "", name: "" };
if (!markerUid || !name) return;
const marker = this.findMarker(markerUid);
if (!marker) return;
this.commitMarkerName(marker, this.findKnownPerson(name), name);
if (markerUid && name) {
const marker = this.findMarker(markerUid);
if (marker) this.commitMarkerName(marker, this.findKnownPerson(name), name);
}
this.resolveAddNameNav();
},
onAddNameCancel() {
const { markerUid } = this.addNameDialog;
@ -1398,6 +1399,14 @@ export default {
draft.current = draft.original || "";
draft.editing = false;
}
this.resolveAddNameNav();
},
// Resolves the nav promise parked by `confirmDiscardPending` when
// Add-name was open both Add and Cancel settle the draft.
resolveAddNameNav() {
const resolve = this._addNameNavResolver;
this._addNameNavResolver = null;
if (resolve) resolve(true);
},
cancelMarkerName(marker) {
if (!marker || !marker.UID) return;
@ -1457,12 +1466,6 @@ export default {
// Fire-and-forget commit of any pending chip removals. Mirrors the
// inline-text auto-commit on blur: the user's intent (clicking ×) is
// honored on navigation/close instead of being silently discarded.
// Each Photo.removeLabel / Photo.removeFromAlbum call captures
// `this.photo` at the time of invocation, so the response patches the
// original Photo instance even if the slide has changed by the time the
// promise resolves. The catch path surfaces `$notify.error` and is
// shared with the manual Confirm path through confirmLabels /
// confirmAlbums.
autoCommitChipRemovals() {
if (this.chipState.labels.removals.length) {
this.confirmLabels();
@ -1471,6 +1474,20 @@ export default {
this.confirmAlbums();
}
},
// Settles dirty marker drafts as if @blur had fired keyboard / code
// navigation skips the input blur, so without this the discard
// dialog would race the eventual commit.
flushDirtyMarkerDrafts() {
Object.keys(this.markerDrafts).forEach((uid) => {
const draft = this.markerDrafts[uid];
if (!draft) return;
const name = this.unwrapMarkerName(draft.current).trim();
const original = (draft.original || "").trim();
if (!name || name === original) return;
const marker = this.findMarker(uid);
if (marker) this.confirmMarkerName(marker, "blur");
});
},
// Async guard used by the lightbox before closing / hiding / navigating.
// Returns a Promise<boolean>: true = safe to proceed, false = user
// canceled. Pending chip removals auto-commit BEFORE the dialog gate,
@ -1479,6 +1496,15 @@ export default {
// dialog) never for chip × clicks, which are deliberate and final.
confirmDiscardPending() {
this.autoCommitChipRemovals();
this.flushDirtyMarkerDrafts();
// Defer to an open Add-name dialog instead of stacking a second
// prompt; Add commits, Cancel discards, both resolve via
// `resolveAddNameNav`.
if (this.addNameDialog?.visible) {
return new Promise((resolve) => {
this._addNameNavResolver = resolve;
});
}
if (!this.hasPendingEdit()) return Promise.resolve(true);
if (this.discardDialog.visible && this.discardDialog.resolver) {
// Another request is already waiting on the dialog; reuse it.

View file

@ -3113,6 +3113,93 @@ describe("PSidebarInfo component", () => {
// Resolve so the test doesn't hang.
w.vm.onDiscardCancel();
});
// Settling matched drafts up front avoids the @blur vs discard-dialog
// race that used to persist the change even when the user clicked Discard.
it("auto-commits a matched marker draft so the discard dialog does not appear", async () => {
const setName = vi.fn().mockResolvedValue(undefined);
const marker = {
UID: "m2",
Name: "",
SubjUID: "",
thumbnailUrl: () => "/t/thumb2/public/tile_160",
setName,
};
const photo = { ...mockPhoto, getMarkers: vi.fn().mockReturnValue([marker]) };
const knownPersonConfig = {
...validationConfig,
values: { people: [{ UID: "sALC", Name: "Alice Smith" }] },
};
const w = mountSidebar({
props: { modelValue: mockModel, photo, canEdit: true, context: contexts.Photos },
global: { stubs: { PMap: true }, mocks: { $config: knownPersonConfig, $util: validationUtil } },
});
await w.vm.$nextTick();
w.vm.setMarkerInputValue("m2", "Alice Smith");
const result = w.vm.confirmDiscardPending();
expect(setName).toHaveBeenCalledTimes(1);
expect(marker.Name).toBe("Alice Smith");
expect(marker.SubjUID).toBe("sALC");
expect(w.vm.discardDialog.visible).toBe(false);
await expect(result).resolves.toBe(true);
});
// Stacking a discard dialog on top of the Add-name dialog would
// double-prompt for the same decision; defer instead.
it("defers to the Add-name dialog instead of stacking a discard prompt", async () => {
const marker = {
UID: "m2",
Name: "",
SubjUID: "",
thumbnailUrl: () => "/t/thumb2/public/tile_160",
setName: vi.fn().mockResolvedValue(undefined),
};
const photo = { ...mockPhoto, getMarkers: vi.fn().mockReturnValue([marker]) };
const w = mountSidebar({
props: { modelValue: mockModel, photo, canEdit: true, context: contexts.Photos },
global: { stubs: { PMap: true }, mocks: { $config: validationConfig, $util: validationUtil } },
});
await w.vm.$nextTick();
w.vm.setMarkerInputValue("m2", "Brand New Name");
w.vm.confirmMarkerName(marker, "blur");
expect(w.vm.addNameDialog.visible).toBe(true);
const result = w.vm.confirmDiscardPending();
// No discard dialog stacked on top of Add-name.
expect(w.vm.discardDialog.visible).toBe(false);
// Cancelling Add-name resolves navigation without a second prompt.
w.vm.onAddNameCancel();
await expect(result).resolves.toBe(true);
});
it("confirming the Add-name dialog also resolves the deferred navigation", async () => {
const setName = vi.fn().mockResolvedValue(undefined);
const marker = {
UID: "m2",
Name: "",
SubjUID: "",
thumbnailUrl: () => "/t/thumb2/public/tile_160",
setName,
};
const photo = { ...mockPhoto, getMarkers: vi.fn().mockReturnValue([marker]) };
const w = mountSidebar({
props: { modelValue: mockModel, photo, canEdit: true, context: contexts.Photos },
global: { stubs: { PMap: true }, mocks: { $config: validationConfig, $util: validationUtil } },
});
await w.vm.$nextTick();
w.vm.setMarkerInputValue("m2", "Brand New Name");
w.vm.confirmMarkerName(marker, "blur");
const result = w.vm.confirmDiscardPending();
w.vm.onAddNameConfirm();
expect(setName).toHaveBeenCalledTimes(1);
expect(marker.Name).toBe("Brand New Name");
await expect(result).resolves.toBe(true);
});
});
// L9: onChipEscape clears the typed text and pending removals for one