From bc077b770e92e56c4a59270c37c5bdc90debf7b7 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 5 Sep 2026 21:01:22 +0200 Subject: [PATCH] =?UTF-8?q?fix(recipes):=20garde=20le=20popover=20de=20cor?= =?UTF-8?q?rection=20ouvert=20pendant=20la=20s=C3=A9lection=20d'un=20span?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Depuis la refonte #29-#32, attacher un ingrédient/ustensile/température à une correction de technique demande de surligner le passage concerné dans la description. Mais le `

` de la description est un frère du popover dans le DOM, et le listener « clic extérieur » du popover (`mousedown` hors de son propre DOM → `onClose`) fermait le popover dès le `mousedown` qui démarre ce glisser-sélectionner — avant même le `mouseup`. Le `mouseup` ouvrait alors un nouveau popover vierge pour le texte sélectionné, traité comme une toute nouvelle proposition de technique (la technique choisie et les métadonnées déjà ajoutées étaient perdues). `TechStepCorrectionPopover` accepte désormais un prop optionnel `descriptionContainerRef` (le `containerRef` que `StepDescription` a déjà) : un `mousedown` à l'intérieur n'est plus un clic extérieur. `StepDescription` pilote déjà explicitement chaque interaction dans la description (`setActiveCorrection`/`setResolvedMetadataSpan`), donc retirer cette fermeture concurrente ne perd rien ; un clic vraiment ailleurs ferme toujours le popover. Comble aussi le trou de couverture que `TechStepCorrectionPopover.cy.tsx` prétendait déjà délégué à un scénario e2e inexistant : nouveau test dans `recipes.cy.ts` avec une vraie sélection de texte (Range/Selection sur les nœuds `data-offset`, séquence mousedown→mouseup réelle) — vérifié qu'il échoue sans le correctif. Closes #36 Co-Authored-By: Claude Sonnet 5 --- apps/web/cypress/e2e/recipes.cy.ts | 96 +++++++++++++++++++ .../recipes/steps/StepDescription.tsx | 1 + .../steps/TechStepCorrectionPopover.tsx | 29 +++++- 3 files changed, 121 insertions(+), 5 deletions(-) diff --git a/apps/web/cypress/e2e/recipes.cy.ts b/apps/web/cypress/e2e/recipes.cy.ts index d3a4958..137dcd9 100644 --- a/apps/web/cypress/e2e/recipes.cy.ts +++ b/apps/web/cypress/e2e/recipes.cy.ts @@ -127,6 +127,45 @@ const tarteDetail = { ], }; +/** + * A one-step recipe whose single technique match carries no attached + * metadata yet — for the issue #36 regression test below, which needs a + * clean correction popover and a description word ("oignons") that appears + * *only* in the step text, nowhere in the popover itself. + */ +const brownStepRecipe = { + id: 6, + name: "Soupe", + description: null, + picture: null, + portions: 4, + authorId: 1, + visibility: "PERSONAL", + allergens: [], + diets: [], + isFavorite: false, + ingredients: [], + steps: [ + { + id: 1, + description: "Faire revenir les oignons.", + picture: null, + order: 1, + techSteps: [ + { + techStep: { id: 3, key: "brown" }, + start: 0, + end: 13, + source: "auto", + ingredients: [], + utensils: [], + temperatures: [], + }, + ], + }, + ], +}; + function interceptAuth() { cy.intercept("GET", "**/auth/me", { statusCode: 200, body: authenticatedProfile }); cy.intercept("GET", "**/profile/disliked-ingredients", { statusCode: 200, body: [] }); @@ -293,6 +332,63 @@ describe("Recipe catalog", () => { }); }); + // Regression (issue #36): while the correction popover's "add an + // ingredient/utensil/temperature" flow waits for a text selection, the + // mousedown that *starts* that drag lands on the step description — a DOM + // sibling of the popover — and used to count as an outside-click, + // closing the popover before mouseup; the selection then opened a fresh + // blank popover for the selected word as a brand-new technique. The + // description container is now exempt from the popover's outside-click. + it("keeps the same correction popover open while selecting a span for an ingredient", () => { + cy.viewport(1600, 900); + cy.intercept("GET", /\/recipes\?/, { statusCode: 200, body: [] }); + cy.intercept("GET", "**/recipes/6", { statusCode: 200, body: brownStepRecipe }); + cy.intercept("GET", "**/reference/tech-steps", { + statusCode: 200, + body: [{ id: 3, key: "brown" }], + }); + cy.intercept("GET", "**/reference/ingredients", { statusCode: 200, body: [] }); + cy.intercept("GET", "**/reference/units", { statusCode: 200, body: [] }); + cy.intercept("GET", "**/reference/utensils", { statusCode: 200, body: [] }); + + cy.visit("/recettes/6"); + + // Open the technique correction, then ask to attach an ingredient — the + // popover now shows the "select the span in the description" hint. + cy.contains(".step-tech-step", "Faire revenir").click(); + cy.get(".tech-step-correction-popover").should("have.length", 1); + cy.get(".tech-step-correction-popover__selection").should("contain.text", "Faire revenir"); + cy.contains(".tech-step-correction-popover__add-row", "Ajouter un ingrédient").click(); + cy.get(".tech-step-correction-popover__hint").should("be.visible"); + + // Reproduce the real mousedown → drag → mouseup over the description. + // The mousedown is the event that used to close the popover. + cy.contains(".recipe-detail-panel__steps p", "Faire revenir").as("stepParagraph"); + cy.get("@stepParagraph").trigger("mousedown"); + cy.window().then((win) => { + const paragraph = win.document.querySelector(".recipe-detail-panel__steps p") as HTMLElement; + const span = [...paragraph.querySelectorAll("[data-offset]")].find((el) => + el.textContent?.includes("oignons"), + ); + const textNode = span?.firstChild as Text; + const local = textNode.textContent?.indexOf("oignons") ?? 0; + const range = win.document.createRange(); + range.setStart(textNode, local); + range.setEnd(textNode, local + "oignons".length); + const selection = win.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); + }); + cy.get("@stepParagraph").trigger("mouseup"); + + // Still the *same* popover — its heading still quotes the technique span + // "Faire revenir", not "oignons" — now showing the ingredient sub-form + // for the freshly selected span, not a blank new-technique popover. + cy.get(".tech-step-correction-popover").should("have.length", 1); + cy.get(".tech-step-correction-popover__selection").should("contain.text", "Faire revenir"); + cy.get(".tech-step-correction-popover__add-form").should("be.visible"); + }); + it("shows a not-found message for a selected id the API rejects", () => { cy.intercept("GET", /\/recipes\?/, { statusCode: 200, body: [] }); cy.intercept("GET", "**/recipes/999", { diff --git a/apps/web/src/features/recipes/steps/StepDescription.tsx b/apps/web/src/features/recipes/steps/StepDescription.tsx index b2209b2..05577b0 100644 --- a/apps/web/src/features/recipes/steps/StepDescription.tsx +++ b/apps/web/src/features/recipes/steps/StepDescription.tsx @@ -337,6 +337,7 @@ export function StepDescription({ existingUtensils={activeStepTechStep?.utensils ?? []} existingTemperatures={activeStepTechStep?.temperatures ?? []} resolvedMetadataSpan={resolvedMetadataSpan} + descriptionContainerRef={containerRef} onRequestSpan={setPendingSpanRequest} onClose={closeActiveCorrection} onSubmitted={handleSubmitted} diff --git a/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx b/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx index e73c142..06be3b9 100644 --- a/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx +++ b/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx @@ -9,7 +9,7 @@ import { type UnitView, type UtensilView, } from "@batch-cooking/shared"; -import { type ReactNode, useEffect, useRef, useState } from "react"; +import { type ReactNode, type RefObject, useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { ApiError, apiClient } from "../../../api/client"; import { errorMessageService } from "../../../services/error-message.service"; @@ -304,6 +304,7 @@ export function TechStepCorrectionPopover({ existingUtensils, existingTemperatures, resolvedMetadataSpan, + descriptionContainerRef, onRequestSpan, onClose, onSubmitted, @@ -334,6 +335,20 @@ export function TechStepCorrectionPopover({ range: TextSelectionRange; text: string; } | null; + /** + * The step description's own container (`StepDescription`'s existing + * `containerRef`). A `mousedown` inside it is *not* treated as an + * outside-click here: while the "attach an ingredient/utensil/ + * temperature" flow is waiting for a selection, the drag that makes that + * selection starts with a `mousedown` on the description — which is a DOM + * sibling of this popover, so without this exemption it closed the + * popover before the `mouseup` even landed, and the selection then opened + * a brand-new technique correction instead (issue #36). `StepDescription` + * already drives every meaningful interaction inside it + * (`setActiveCorrection`/`setResolvedMetadataSpan`) explicitly, so + * dropping the close here loses nothing. + */ + descriptionContainerRef?: RefObject; /** Tells `StepDescription` "the next text selection in the description is for an ingredient/utensil/temperature mention, not a new technique correction" — see this component's own doc comment. */ onRequestSpan: (kind: MetadataKind) => void; onClose: () => void; @@ -459,13 +474,17 @@ export function TechStepCorrectionPopover({ useEffect(() => { function handleClickOutside(e: MouseEvent) { - if (popoverRef.current && !popoverRef.current.contains(e.target as Node)) { - onClose(); - } + const target = e.target as Node; + if (popoverRef.current?.contains(target)) return; + // A mousedown in the description isn't an "outside" click — it's how + // the viewer selects the span for an ingredient/utensil/temperature + // mention. Let StepDescription handle it (see `descriptionContainerRef`). + if (descriptionContainerRef?.current?.contains(target)) return; + onClose(); } document.addEventListener("mousedown", handleClickOutside); return () => document.removeEventListener("mousedown", handleClickOutside); - }, [onClose]); + }, [onClose, descriptionContainerRef]); async function removeMatch() { setIsSubmitting(true);