From d924c665f8d864ad98c1bd1dd4356bb0cd3ba06a Mon Sep 17 00:00:00 2001 From: npeter83 Date: Mon, 21 Sep 2026 23:59:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(frontend):=20round=202=20=E2=80=94=20the=20?= =?UTF-8?q?popover=20latch=20reset=20read=20a=20stale,=20negative=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reset I added guards on `hidden === 0`, and `hidden` can be NEGATIVE. `useChipOverflowFit` keeps `visible` in state and re-decides in a layout effect, so the render in which the chip set SHRINKS still carries the previous measurement — delete three tags under an open "+N" and `chips.length - visible` goes to -2, the equality never matches, and the flag survives the one window the reset exists to close. The next re-fit brings the menu back on its own, which is the original defect with an extra step. Clamped at all three lanes, with a test that drives the shrink through the stale value. Also: the Scheduler characterization banner said "the four tests below" and now sits above six, the last two of which are genuine regression guards for the range-refusal fix. A future mutation check would have read it, seen them go red, and concluded the harness was broken rather than that the guard worked. --- frontend/src/components/ChannelTags.tsx | 14 +++++++++-- .../src/components/Scheduler.dom.test.tsx | 12 ++++++---- .../components/TagFilterChips.dom.test.tsx | 23 +++++++++++++++++++ frontend/src/components/TagFilterChips.tsx | 7 +++++- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index 95c3a5b0..d3a42483 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -117,7 +117,12 @@ function SuggestedChipsLane({ decide: (g) => fitChipsWithPill({ chipRights: g.chipRights, pillW: g.pillW, avail: g.avail, gap: g.gap }), }); - const hidden = suggestions.length - visible; + // Clamped, and not defensively: `useChipOverflowFit` keeps `visible` in state and re-decides in a + // layout effect, so the render in which the chip set SHRINKS still holds the previous measurement + // and a raw subtraction goes negative. An unclamped `hidden` slips past the `=== 0` reset below — + // the flag survives exactly the window it exists to close, and the popover springs back on the + // next re-fit. + const hidden = Math.max(0, suggestions.length - visible); // Open only while there is a "+N" trigger to anchor it — derived, not closed by an effect, so the // frame in which the last collapsed chip fits again cannot also hold a menu pointing at a @@ -345,7 +350,12 @@ export function ChannelTagsControl({ fitChipsWithPill({ chipRights: g.chipRights, pillW: g.pillW, avail: g.avail, gap: g.gap }), }); - const hidden = attached.length - visible; + // Clamped, and not defensively: `useChipOverflowFit` keeps `visible` in state and re-decides in a + // layout effect, so the render in which the chip set SHRINKS still holds the previous measurement + // and a raw subtraction goes negative. An unclamped `hidden` slips past the `=== 0` reset below — + // the flag survives exactly the window it exists to close, and the popover springs back on the + // next re-fit. + const hidden = Math.max(0, attached.length - visible); const [editorOpen, setEditorOpen] = useState(false); const editPop = useAnchoredPopover({ diff --git a/frontend/src/components/Scheduler.dom.test.tsx b/frontend/src/components/Scheduler.dom.test.tsx index c8d63ad8..512d1980 100644 --- a/frontend/src/components/Scheduler.dom.test.tsx +++ b/frontend/src/components/Scheduler.dom.test.tsx @@ -80,11 +80,13 @@ afterEach(() => { const intervalBox = () => screen.queryByRole("spinbutton", { name: "" }); const batchBox = () => screen.getByLabelText("scheduler.maintenance.batchLabel"); -// The four tests below are CHARACTERIZATION, not regression guards for E27 S2: the old JobRow's -// re-sync effect was already guarded by `if (!editing)` and the mirror only rendered while editing, -// so every one of them passes against the pre-sweep code too. They are here because the editor had -// no tests at all. The editor that genuinely lost the user's typing to a poll is the maintenance -// batch box below — its old effect had no guard — and that one has its own test. +// The FIRST FOUR tests below are CHARACTERIZATION, not regression guards for E27 S2: the old +// JobRow's re-sync effect was already guarded by `if (!editing)` and the mirror only rendered while +// editing, so each of those four passes against the pre-sweep code too. They are here because the +// editor had no tests at all. The two after them — the range refusal and the focus return — ARE +// regression guards, for the silent-discard fix, and they fail against the code this file shipped +// with. The editor that genuinely lost the user's typing to a poll is the maintenance batch box +// below: its old effect had no guard, and that one has its own test. describe("Scheduler · the job interval editor", () => { it("opens on the current interval and commits a new one", async () => { const update = vi.spyOn(api, "updateSchedulerJob").mockResolvedValue(undefined as never); diff --git a/frontend/src/components/TagFilterChips.dom.test.tsx b/frontend/src/components/TagFilterChips.dom.test.tsx index 9782c8d9..926c067e 100644 --- a/frontend/src/components/TagFilterChips.dom.test.tsx +++ b/frontend/src/components/TagFilterChips.dom.test.tsx @@ -66,6 +66,29 @@ describe("TagFilterChips — the overflow menu", () => { expect(moreBtn()).toHaveProperty("ariaExpanded", "false"); }); + it("does not come back when the chip set SHRINKS under the open menu", () => { + // The nastier half of the same bug. `useChipOverflowFit` holds `visible` in state and re-decides + // in a layout effect, so the render in which the set shrinks still carries the PREVIOUS + // measurement — `chips.length - visible` goes negative there, and a `hidden === 0` guard reading + // an unclamped value never fires. The flag then survives the one window it exists to close. + const four = [...chips, chip("delta")]; + fit.visible = 1; + const { rerender } = render(); + fireEvent.click(moreBtn()!); + expect(menu()).toBeTruthy(); + + // Three tags are deleted in the tag manager. `visible` is still the stale 3. + fit.visible = 3; + rerender(); + expect(menu()).toBeNull(); + + // …and now the layout effect lands: two long names, only one fits. + fit.visible = 1; + rerender(); + expect(menu()).toBeNull(); + expect(moreBtn()).toHaveProperty("ariaExpanded", "false"); + }); + it("still opens and closes on the trigger", () => { render(); fireEvent.click(moreBtn()!); diff --git a/frontend/src/components/TagFilterChips.tsx b/frontend/src/components/TagFilterChips.tsx index d4e9359b..c9ba083e 100644 --- a/frontend/src/components/TagFilterChips.tsx +++ b/frontend/src/components/TagFilterChips.tsx @@ -58,7 +58,12 @@ export default function TagFilterChips({ fitChipsWithPill({ chipRights: g.chipRights, pillW: g.pillW, avail: g.avail, gap: g.gap }), }); - const hidden = chips.length - visible; + // Clamped, and not defensively: `useChipOverflowFit` keeps `visible` in state and re-decides in a + // layout effect, so the render in which the chip set SHRINKS still holds the previous measurement + // and a raw subtraction goes negative. An unclamped `hidden` slips past the `=== 0` reset below — + // the flag survives exactly the window it exists to close, and the popover springs back on the + // next re-fit. + const hidden = Math.max(0, chips.length - visible); // The popover can only be open while there is a "+N" trigger to anchor it. Derived rather than // closed by an effect, so the frame in which the last overflowing chip fits again cannot also // contain a menu hanging off a reference that is already gone.