Correction tech-steps : sélectionner du texte pour ajouter un ingrédient/ustensile/température ferme le popover en cours #36

New issue

Closed

opened 2026-09-05 14:50:39 +02:00 by kyuno · 0 comments

kyuno commented

2026-09-05 14:50:39 +02:00

Owner

Contexte

Depuis la refonte du popover de correction des tech-steps (#29-#32), une fois
un popover ouvert sur une technique, chaque section Ingrédients/Ustensiles/
Température permet d'ajouter une mention en cliquant "+ Ajouter..." puis en
surlignant le texte correspondant dans la description de l'étape — pensé
explicitement pour ne pas fermer le popover pendant cette sélection (voir
la doc de TechStepCorrectionPopover.tsx : "the description stays fully
visible and selectable the whole time, nothing overlays it" / "can ask the
viewer to select a second span of text without closing this popover
first").

En pratique ce n'est pas ce qui se passe : surligner le texte demandé ferme
le popover ouvert et un autre s'ouvre à sa place — comme si le texte
sélectionné devait servir à une toute nouvelle correction de technique, pas à
l'ingrédient/ustensile/température qu'on essayait d'ajouter à la correction
en cours (perdant au passage la technique déjà choisie et tout ce qui avait
déjà été ajouté avant validation).

Cause

TechStepCorrectionPopover.tsx ferme le popover au premier mousedown en
dehors de son propre DOM (même patron "outside-click" que WeekNavigator's
CalendarPopover) :

// TechStepCorrectionPopover.tsx:460-468
useEffect(() => {
  function handleClickOutside(e: MouseEvent) {
    if (popoverRef.current && !popoverRef.current.contains(e.target as Node)) {
      onClose();
    }
  }
  document.addEventListener("mousedown", handleClickOutside);
  return () => document.removeEventListener("mousedown", handleClickOutside);
}, [onClose]);

Le  contenant la description de l'étape (StepDescription.tsx, ref
containerRef) est un frère du popover dans le DOM, pas un de ses
descendants — donc tout mousedown dedans, y compris celui qui démarre le
glisser-sélectionner demandé par le hint "Sélectionnez le texte
correspondant..." (selectSpanHint), est vu comme "en dehors" et ferme
immédiatement le popover, avant même que le mouseup qui termine la
sélection ne se déclenche.

Séquence exacte :

Clic sur "+ Ajouter un ingrédient" → requestSpan("ingredient")
(TechStepCorrectionPopover.tsx:516-519) → onRequestSpan("ingredient")
remonte à StepDescription, qui met pendingSpanRequest = "ingredient"
(branché directement : onRequestSpan={setPendingSpanRequest},
StepDescription.tsx:340).
L'utilisateur commence à surligner le mot cible dans la description →
mousedown sur le  → handleClickOutside du popover (toujours
monté à cet instant) le voit comme un clic extérieur → onClose() →
closeActiveCorrection() (StepDescription.tsx:140-144) remet tout à
null, y compris pendingSpanRequest → le popover se démonte.
L'utilisateur relâche le clic → mouseup → handleMouseUp s'exécute
toujours (attaché au , pas au popover) mais pendingSpanRequest est
déjà null (remis à zéro à l'étape 2) → tombe dans la branche par défaut,
setActiveCorrection({ range, selectedText, previousTechStepId: null })
(StepDescription.tsx:161-165) → un nouveau popover s'ouvre,
traitant le texte sélectionné comme une toute nouvelle proposition de
technique.

Reproduction

Ouvrir une recette avec au moins une étape (/recettes/:id), sur un texte
contenant à la fois une technique et un ingrédient/ustensile mentionnés
(ex. "Faire revenir l'oignon dans une poêle").
Cliquer sur le mot surlignant une technique (ou sélectionner un bout de
texte pour en proposer une) → le popover de correction s'ouvre.
Choisir une technique si besoin, puis cliquer "+ Ajouter un ingrédient"
(ou Ustensile/Température) — le popover affiche l'invite "Sélectionnez le
texte correspondant dans la description".
Surligner le mot cible dans la description (ex. "oignon") à la souris.
Constat : le popover se ferme et un autre s'ouvre pour "oignon" en
tant que nouvelle proposition de technique — la technique choisie à
l'étape 3 et tout ingrédient/ustensile/température déjà ajouté sont
perdus.

Impact

Rend "ajouter un ingrédient/ustensile/température à une correction de
technique" essentiellement inutilisable — c'est le seul moyen prévu de le
faire (le hint de l'interface le demande explicitement), et il casse
systématiquement à chaque tentative, quelle que soit la section
(Ingrédients/Ustensiles/Température, les trois partagent le même mécanisme
awaitingSpanFor/pendingSpanRequest).

Pourquoi ça n'est couvert par aucun test existant

TechStepCorrectionPopover.cy.tsx (test composant) simule le retour de
onRequestSpan en injectant directement un resolvedMetadataSpan fixe
(Harness), sans jamais faire vivre StepDescription/le vrai DOM de
sélection — avec ce commentaire explicite dans le fichier :

"A real StepDescription resolves onRequestSpan into a fresh
resolvedMetadataSpan via an actual browser text selection — out of scope
for a component test of the popover alone (covered by the e2e scenario
instead)."

Or ce scénario e2e n'existe pas : ni recipes.feature ni recipes.ts ne
contiennent le moindre step lié à l'ajout d'un ingrédient/ustensile/
température via sélection. L'intégration réelle "sélectionner du texte dans
StepDescription pendant qu'un popover TechStepCorrectionPopover est
ouvert" n'est testée nulle part dans la suite — ce qui explique que la
régression ait pu passer inaperçue lors de la vérification de #29/#30 (suite
verte à 100% par ailleurs, mais sur des chemins qui ne couvrent pas cette
interaction précise).

Pistes de correction

Fix recommandé — exempter le conteneur de la description du "clic
extérieur" du popover, plutôt que fermer sur tout mousedown hors de son
propre DOM :

// TechStepCorrectionPopover.tsx
function handleClickOutside(e: MouseEvent) {
  const target = e.target as Node;
  if (popoverRef.current?.contains(target)) return;
  if (descriptionContainerRef?.current?.contains(target)) return; // laisse StepDescription gérer ce cas lui-même
  onClose();
}

descriptionContainerRef : nouveau prop optionnel — StepDescription passe
simplement son containerRef existant (déjà utilisé par
useTextSelection), aucune nouvelle ref à créer, juste un fil à tirer d'un
composant à l'autre.

Vérifié que ça ne casse rien d'autre :

Sélection pour une nouvelle correction de technique (aucun popover
ouvert) : inchangé, ce listener n'existe pas tant qu'aucun popover n'est
monté.
Clic sur une autre technique/température déjà surlignée dans la même
description pendant qu'un popover est ouvert : StepDescription's propre
onClick (StepDescription.tsx:304-320 pour une technique) fait déjà
setActiveCorrection(...) explicitement — inchangé, cette exemption ne
fait que retirer une fermeture concurrente qui arrivait à la même
conclusion par coïncidence.
Clic vraiment hors de la zone (ailleurs sur la page) : toujours traité
comme "outside", popover toujours fermé normalement.

Alternative plus simple, moins fidèle à l'intention documentée : ne
fermer sur aucun clic extérieur tant que awaitingSpanFor !== null (state
déjà existant dans le popover), sans threader de ref supplémentaire — mais
bloque aussi la fermeture pour un clic voulu ailleurs pendant qu'on attend
une sélection (il faudrait alors explicitement cliquer "Annuler" dans la
section pour se débloquer).

Test à ajouter (comble le vrai trou de couverture identifié plus haut) :
écrire le scénario e2e que le commentaire de TechStepCorrectionPopover.cy.tsx
prétend déjà exister (ou corriger ce commentaire si la couverture reste
volontairement au niveau composant) : ouvrir une vraie StepDescription
éditable, ouvrir une correction, cliquer "+ Ajouter un ingrédient", puis
simuler une vraie sélection de texte dans la description et vérifier que
le même popover reste ouvert avec le nouvel ingrédient ajouté (pas un
popover vierge pour une nouvelle technique). Simuler une sélection réelle en
Cypress : .trigger("mousedown") sur le segment de départ, construire un
Range/Selection via cy.window() (document.createRange() +
window.getSelection().addRange(...) sur les vrais nœuds texte des segments
data-offset rendus), puis .trigger("mouseup") sur la description — pas un
simple .click(), qui ne reproduit pas la séquence mousedown-avant-mouseup
qui déclenche ce bug. Sans ce test, rien ne garantit que le correctif tienne
dans la durée (même levier que la régression resourceType de #32).

Hors scope

Toute refonte plus large du flux (ex. ancrer le popover à la position de la
sélection plutôt que de le garder fixe sous l'étape) — le placement actuel
est un choix assumé documenté dans le composant, non remis en cause ici.

🤖 Filed by Claude Code

## Contexte Depuis la refonte du popover de correction des tech-steps (#29-#32), une fois un popover ouvert sur une technique, chaque section Ingrédients/Ustensiles/ Température permet d'ajouter une mention en cliquant "+ Ajouter..." puis en surlignant le texte correspondant dans la description de l'étape — pensé explicitement pour **ne pas** fermer le popover pendant cette sélection (voir la doc de `TechStepCorrectionPopover.tsx` : *"the description stays fully visible and selectable the whole time, nothing overlays it"* / *"can ask the viewer to select a second span of text without closing this popover first"*). En pratique ce n'est pas ce qui se passe : surligner le texte demandé ferme le popover ouvert et **un autre s'ouvre à sa place** — comme si le texte sélectionné devait servir à une toute nouvelle correction de technique, pas à l'ingrédient/ustensile/température qu'on essayait d'ajouter à la correction en cours (perdant au passage la technique déjà choisie et tout ce qui avait déjà été ajouté avant validation). ## Cause `TechStepCorrectionPopover.tsx` ferme le popover au premier `mousedown` en dehors de son propre DOM (même patron "outside-click" que `WeekNavigator`'s `CalendarPopover`) : ```ts // TechStepCorrectionPopover.tsx:460-468 useEffect(() => { function handleClickOutside(e: MouseEvent) { if (popoverRef.current && !popoverRef.current.contains(e.target as Node)) { onClose(); } } document.addEventListener("mousedown", handleClickOutside); return () => document.removeEventListener("mousedown", handleClickOutside); }, [onClose]); ``` Le `` contenant la description de l'étape (`StepDescription.tsx`, ref `containerRef`) est un **frère** du popover dans le DOM, pas un de ses descendants — donc *tout* `mousedown` dedans, y compris celui qui démarre le glisser-sélectionner demandé par le hint "Sélectionnez le texte correspondant..." (`selectSpanHint`), est vu comme "en dehors" et ferme immédiatement le popover, **avant même que le `mouseup` qui termine la sélection ne se déclenche**. Séquence exacte : 1. Clic sur "+ Ajouter un ingrédient" → `requestSpan("ingredient")` (`TechStepCorrectionPopover.tsx:516-519`) → `onRequestSpan("ingredient")` remonte à `StepDescription`, qui met `pendingSpanRequest = "ingredient"` (branché directement : `onRequestSpan={setPendingSpanRequest}`, `StepDescription.tsx:340`). 2. L'utilisateur commence à surligner le mot cible dans la description → `mousedown` sur le `` → `handleClickOutside` du popover (toujours monté à cet instant) le voit comme un clic extérieur → `onClose()` → `closeActiveCorrection()` (`StepDescription.tsx:140-144`) remet **tout** à `null`, y compris `pendingSpanRequest` → le popover se démonte. 3. L'utilisateur relâche le clic → `mouseup` → `handleMouseUp` s'exécute toujours (attaché au ``, pas au popover) mais `pendingSpanRequest` est déjà `null` (remis à zéro à l'étape 2) → tombe dans la branche par défaut, `setActiveCorrection({ range, selectedText, previousTechStepId: null })` (`StepDescription.tsx:161-165`) → **un nouveau** popover s'ouvre, traitant le texte sélectionné comme une toute nouvelle proposition de technique. ## Reproduction 1. Ouvrir une recette avec au moins une étape (`/recettes/:id`), sur un texte contenant à la fois une technique et un ingrédient/ustensile mentionnés (ex. "Faire revenir l'oignon dans une poêle"). 2. Cliquer sur le mot surlignant une technique (ou sélectionner un bout de texte pour en proposer une) → le popover de correction s'ouvre. 3. Choisir une technique si besoin, puis cliquer "+ Ajouter un ingrédient" (ou Ustensile/Température) — le popover affiche l'invite "Sélectionnez le texte correspondant dans la description". 4. Surligner le mot cible dans la description (ex. "oignon") à la souris. 5. **Constat** : le popover se ferme et un autre s'ouvre pour "oignon" en tant que nouvelle proposition de technique — la technique choisie à l'étape 3 et tout ingrédient/ustensile/température déjà ajouté sont perdus. ## Impact Rend "ajouter un ingrédient/ustensile/température à une correction de technique" essentiellement inutilisable — c'est le **seul** moyen prévu de le faire (le hint de l'interface le demande explicitement), et il casse systématiquement à chaque tentative, quelle que soit la section (Ingrédients/Ustensiles/Température, les trois partagent le même mécanisme `awaitingSpanFor`/`pendingSpanRequest`). ## Pourquoi ça n'est couvert par aucun test existant `TechStepCorrectionPopover.cy.tsx` (test composant) simule le retour de `onRequestSpan` en injectant directement un `resolvedMetadataSpan` fixe (`Harness`), sans jamais faire vivre `StepDescription`/le vrai DOM de sélection — avec ce commentaire explicite dans le fichier : > "A real `StepDescription` resolves `onRequestSpan` into a fresh > `resolvedMetadataSpan` via an actual browser text selection — out of scope > for a component test of the popover alone (**covered by the e2e scenario > instead**)." Or ce scénario e2e n'existe pas : ni `recipes.feature` ni `recipes.ts` ne contiennent le moindre step lié à l'ajout d'un ingrédient/ustensile/ température via sélection. L'intégration réelle "sélectionner du texte dans `StepDescription` pendant qu'un popover `TechStepCorrectionPopover` est ouvert" n'est testée nulle part dans la suite — ce qui explique que la régression ait pu passer inaperçue lors de la vérification de #29/#30 (suite verte à 100% par ailleurs, mais sur des chemins qui ne couvrent pas cette interaction précise). ## Pistes de correction **Fix recommandé** — exempter le conteneur de la description du "clic extérieur" du popover, plutôt que fermer sur tout `mousedown` hors de son propre DOM : ```ts // TechStepCorrectionPopover.tsx function handleClickOutside(e: MouseEvent) { const target = e.target as Node; if (popoverRef.current?.contains(target)) return; if (descriptionContainerRef?.current?.contains(target)) return; // laisse StepDescription gérer ce cas lui-même onClose(); } ``` `descriptionContainerRef` : nouveau prop optionnel — `StepDescription` passe simplement son `containerRef` existant (déjà utilisé par `useTextSelection`), aucune nouvelle ref à créer, juste un fil à tirer d'un composant à l'autre. Vérifié que ça ne casse rien d'autre : - Sélection pour une **nouvelle** correction de technique (aucun popover ouvert) : inchangé, ce listener n'existe pas tant qu'aucun popover n'est monté. - Clic sur une autre technique/température déjà surlignée dans la même description pendant qu'un popover est ouvert : `StepDescription`'s propre `onClick` (`StepDescription.tsx:304-320` pour une technique) fait déjà `setActiveCorrection(...)` explicitement — inchangé, cette exemption ne fait que retirer une fermeture concurrente qui arrivait à la même conclusion par coïncidence. - Clic vraiment hors de la zone (ailleurs sur la page) : toujours traité comme "outside", popover toujours fermé normalement. **Alternative plus simple, moins fidèle à l'intention documentée** : ne fermer sur aucun clic extérieur tant que `awaitingSpanFor !== null` (state déjà existant dans le popover), sans threader de ref supplémentaire — mais bloque aussi la fermeture pour un clic voulu ailleurs pendant qu'on attend une sélection (il faudrait alors explicitement cliquer "Annuler" dans la section pour se débloquer). **Test à ajouter** (comble le vrai trou de couverture identifié plus haut) : écrire le scénario e2e que le commentaire de `TechStepCorrectionPopover.cy.tsx` prétend déjà exister (ou corriger ce commentaire si la couverture reste volontairement au niveau composant) : ouvrir une vraie `StepDescription` éditable, ouvrir une correction, cliquer "+ Ajouter un ingrédient", puis simuler une **vraie** sélection de texte dans la description et vérifier que le **même** popover reste ouvert avec le nouvel ingrédient ajouté (pas un popover vierge pour une nouvelle technique). Simuler une sélection réelle en Cypress : `.trigger("mousedown")` sur le segment de départ, construire un `Range`/`Selection` via `cy.window()` (`document.createRange()` + `window.getSelection().addRange(...)` sur les vrais nœuds texte des segments `data-offset` rendus), puis `.trigger("mouseup")` sur la description — pas un simple `.click()`, qui ne reproduit pas la séquence mousedown-avant-mouseup qui déclenche ce bug. Sans ce test, rien ne garantit que le correctif tienne dans la durée (même levier que la régression `resourceType` de #32). ## Hors scope - Toute refonte plus large du flux (ex. ancrer le popover à la position de la sélection plutôt que de le garder fixe sous l'étape) — le placement actuel est un choix assumé documenté dans le composant, non remis en cause ici. --- 🤖 Filed by [Claude Code](https://claude.com/claude-code)