diff --git a/src/features/buddy/actionDrafts.ts b/src/features/buddy/actionDrafts.ts new file mode 100644 index 00000000..2339f5ed --- /dev/null +++ b/src/features/buddy/actionDrafts.ts @@ -0,0 +1,20 @@ +/** + * The hire's own wording for a proposal that carries an editable message, held per action. + * + * Deliberately *not* inside the card that renders the field. The dock unmounts every time it is + * closed, and the hand-off to `/buddy` mounts a second card for the same action — so a draft kept + * as component state is thrown away by gestures that were never about the text. Held by the + * session instead, the way the composer's own draft already is, the wording outlives whichever + * surface happens to be on screen. + */ +export type ActionDrafts = Record; + +/** + * How one draft is keyed: an action inside the message it was proposed in. + * + * Both halves are needed. A hire offer's id is local to its message (the backend assigns none), so + * keying by id alone would let one message's wording surface in another's card. + */ +export function actionDraftKey(messageId: string, actionId: string): string { + return `${messageId}:${actionId}`; +} diff --git a/src/features/buddy/components/BuddyActionProposals.tsx b/src/features/buddy/components/BuddyActionProposals.tsx index 9c28b4ec..5b07f12e 100644 --- a/src/features/buddy/components/BuddyActionProposals.tsx +++ b/src/features/buddy/components/BuddyActionProposals.tsx @@ -1,4 +1,8 @@ -import { Check, Info, Loader2, RotateCcw, TriangleAlert, Users, X } from "lucide-react"; +import { AlertCircle, Check, Info, Loader2, TriangleAlert, Users, X } from "lucide-react"; +import { Field } from "../../../components/ui/Field"; +import { Textarea } from "../../../components/ui/Textarea"; +import { actionDraftKey } from "../actionDrafts"; +import type { ActionDrafts } from "../actionDrafts"; import type { ProposedAction, ProposalRisk } from "../types"; import { BUDDY_ACTION_AMEND_CHECKLIST, @@ -16,6 +20,19 @@ type BuddyActionProposalsProps = { /** The message these actions were proposed in — needed to target the confirm. */ messageId: string; actions: ProposedAction[]; + /** The session's wording for the offers that carry an editable message — see `actionDrafts`. */ + actionDrafts: ActionDrafts; + /** Records one, so it outlives whichever surface is on screen. */ + setActionDraft: (key: string, text: string) => void; + onConfirm: (messageId: string, action: ProposedAction) => void; + onDismiss: (messageId: string, action: ProposedAction) => void; +}; + +type BuddyProposalCardProps = { + messageId: string; + action: ProposedAction; + actionDrafts: ActionDrafts; + setActionDraft: (key: string, text: string) => void; onConfirm: (messageId: string, action: ProposedAction) => void; onDismiss: (messageId: string, action: ProposedAction) => void; }; @@ -35,11 +52,20 @@ type BuddyActionProposalsProps = { * permanent, and it was badly wrong when it was not — a hire whose packet failed to assemble had * the only control for it taken away, and the mentor, which is never told what became of a * proposal, went on asking them to click a button that was no longer on screen. So the outcome - * stays, and the offer comes back under it. + * stays, and the offer comes back under it — whole, field and all, because a flag the backend + * could not send is precisely the one whose wording is worth another look. + * + * Each one is its own {@link BuddyProposalCard} rather than an inline branch of the map below: a + * flag to the PM carries the hire's own words, and those are edited **before** confirming, so the + * offer is a component with an identity of its own rather than a row inside the callback — and what + * the hire typed is held by the **session** (`actionDrafts`), not by the card, so closing the dock + * or handing the conversation over to `/buddy` cannot throw away a half-worded question. */ export function BuddyActionProposals({ messageId, actions, + actionDrafts, + setActionDraft, onConfirm, onDismiss, }: BuddyActionProposalsProps) { @@ -47,130 +73,199 @@ export function BuddyActionProposals({ return (
- {actions.map((action) => { - if (action.status === "dismissed") { - return ( -

- Dismissed — nothing changed. -

- ); - } + {actions.map((action) => ( + + ))} +
+ ); +} - if (action.status === "resolved") { - return ( -
-

-

- {/* Under the refusal, not instead of it. The outcome is why it did not work and is - the thing worth reading; this is only the way to try it once the reason has been - dealt with. Never offered on success — running a confirmed action twice is how - somebody claims the same task twice. Hire actions only: a stored team - proposal is settled server-side once decided, so confirming it again - could only be refused. */} - {!action.ok && "action" in action && ( - - )} - {/* Opening orientation is the one hire action whose result is content, not - just an outcome line: the packet renders right here in the thread - instead of navigating to a page. Stored proposals have no `action` - name to match — their payoff is always the outcome line. */} - {"action" in action && - action.action === BUDDY_ACTION_OPEN_ORIENTATION && - action.ok && } -
- ); - } +/** + * One proposed action, from offer to settled — and the only place the hire's own words can be + * edited before they leave the product. + * + * A `flag_to_pm` is the one proposal whose payload is a message a person reads: the buddy composes + * the question, but it arrives in a PM's inbox in the hire's name, so the whole question is shown + * in a field above the button and is theirs to correct. What is confirmed is therefore exactly what + * the field held (see `handleConfirm`), never the original proposal. Every other action echoes a + * target back verbatim and a stored team proposal confirms by id — nothing there is editable, so + * nothing there grows a field. + */ +function BuddyProposalCard({ + messageId, + action, + actionDrafts, + setActionDraft, + onConfirm, + onDismiss, +}: BuddyProposalCardProps) { + const isStored = "proposalId" in action; + const isFlagToPm = !isStored && action.action === BUDDY_ACTION_FLAG_TO_PM; + const isConfirming = action.status === "confirming"; + const draftKey = actionDraftKey(messageId, action.id); + // The hire's own words for this offer, if they have touched it — read from the session, never + // from state of this card's own: the dock unmounts when it is closed and `/buddy` mounts a + // second card for the same action, so wording kept here would be thrown away by either gesture. + // Until they edit, the field holds the question the buddy composed. + const editedQuestion = actionDrafts[draftKey] ?? (isFlagToPm ? (action.question ?? "") : ""); + /** A flag with nothing in it is not a question — the confirm stays out of reach rather than + * letting the backend answer with a refusal the hire cannot act on. The field says so out + * loud (`error` below): a disabled button is not a reason, and a screen reader gets nothing + * from it. */ + const hasQuestion = editedQuestion.trim().length > 0; + const canConfirm = !isConfirming && (!isFlagToPm || hasQuestion); - const isConfirming = action.status === "confirming"; - const isStored = "proposalId" in action; - // Fail closed: a proposal whose risk did not survive the stream (or that arrived - // without its description) is not confirmable — an approval card for a project - // mutation must never guess how loudly to warn, nor confirm what it cannot show. A - // hire's board edit follows the same rule: no confirm for a change it cannot show. - const isUnsupported = isStored - ? action.risk === null || !action.preview - : lacksBoardEditDetails(action); - const previewId = `buddy-proposal-preview-${action.id}`; + /** The card's only way out: the offer's button before it has run, and the very same button after + * a refusal. Both come through here, so neither can send a question other than the one on + * screen — and never an empty one. */ + const handleConfirm = () => { + if (isFlagToPm) { + const question = editedQuestion.trim(); + if (!question) return; + // What was sent is what the field now holds; only the trim can differ. A refused retry + // has to hand the wording back exactly as the PM would have read it, not padded. + if (question !== editedQuestion) setActionDraft(draftKey, question); + onConfirm(messageId, { ...action, question }); + return; + } - return ( -
- {/* What confirming would change comes FIRST, in the backend's own words, and the - confirm button describes it (aria-describedby). For a stored team proposal the - target is the id, so this is the offer's full description; for a hire's board edit - it is the part the payload alone would not show — which cards, which lines go. - Never a summary the client recomposed, and never read after the button. */} - {action.preview && ( -

- {action.preview} -

- )} + onConfirm(messageId, action); + }; + + if (action.status === "dismissed") { + return

Dismissed — nothing changed.

; + } + + // A hire offer the backend could not run is **not** spent: it keeps its card — reason above it, + // field and buttons as they were — so the hire can act on what the refusal said. That matters + // most for a flag, whose wording is the one thing on the card they can correct. A success *is* + // spent (running a confirmed action twice is how somebody claims the same task twice), and a + // stored team proposal is settled server-side once decided. + const wasRefused = action.ok === false && action.outcome !== undefined; + const isSettled = action.status === "resolved" && (action.ok === true || isStored); + + // Kept above the card while a retry is on its way, not just when the refusal arrives: pressing + // the button again must not blank out the sentence explaining why it did not work. The one state + // that drops it is a transport error *after* such a retry — the card's own note below is the + // newer story, and two messages about one press read as two failures. + const outcomeLine = + action.status === "resolved" || (wasRefused && action.status !== "error") ? ( +

+ {action.ok ? ( +

+ ) : null; + + if (isSettled) { + return ( +
+ {outcomeLine} + {/* Opening orientation is the one hire action whose result is content, not just an + outcome line: the packet renders right here in the thread instead of navigating to a + page. Stored proposals have no `action` name to match — their payoff is always the + outcome line. */} + {"action" in action && action.action === BUDDY_ACTION_OPEN_ORIENTATION && action.ok && ( + + )} +
+ ); + } + + // Fail closed: a proposal whose risk did not survive the stream (or that arrived + // without its description) is not confirmable — an approval card for a project + // mutation must never guess how loudly to warn, nor confirm what it cannot show. A + // hire's board edit follows the same rule: no confirm for a change it cannot show. + const isUnsupported = isStored + ? action.risk === null || !action.preview + : lacksBoardEditDetails(action); + const previewId = `buddy-proposal-preview-${action.id}`; + + return ( + <> + {/* Under the refusal, never instead of it: the outcome says why it did not work, and the + card below is how it is tried again — whole, because a flag that came back unsent is + exactly the one whose wording is worth another look. */} + {outcomeLine} +
+ {/* What confirming would change comes FIRST, in the backend's own words, and the + confirm button describes it (aria-describedby). For a stored team proposal the + target is the id, so this is the offer's full description; for a hire's board edit + it is the part the payload alone would not show — which cards, which lines go. + Never a summary the client recomposed, and never read after the button. */} + {action.preview && ( +

+ {action.preview} +

+ )} + + {/* Shown before the press, not after it, and only where the payload is *content*. - {/* Shown before the press, not after it, and only where the payload is *content*. `place_checklist` is the one action whose confirm writes the mentor's own sentences onto a surface the hire owns — the same reason the assessment proposal names the skill and the level rather than saying "Save this". The lines are in the reply above as well; having them here is what makes the two comparable, so a list that does not match what was written is visible before it is kept, not after. */} - {!isStored && action.checklistItems && action.checklistItems.length > 0 && ( -
- {action.checklistTitle && ( -

- {action.checklistTitle} -

- )} - {/* Named as an addition when it is one. The card keeps everything it already has, + {!isStored && action.checklistItems && action.checklistItems.length > 0 && ( +
+ {action.checklistTitle && ( +

+ {action.checklistTitle} +

+ )} + {/* Named as an addition when it is one. The card keeps everything it already has, and these lines go after it — saying so is the difference between agreeing to three new steps and agreeing to whatever the list becomes. */} - {action.action === BUDDY_ACTION_AMEND_CHECKLIST && ( -

- Added to the end of that list — nothing on it changes: -

- )} - {action.action === BUDDY_ACTION_TICK_CHECKLIST && ( -

- Ticked off on that list — nothing else on it changes: -

- )} - {/* The whole list as it would read, replacing what is there. What would be lost is - named in the preview above, since a missing line is invisible in this one. */} - {action.action === BUDDY_ACTION_EDIT_CHECKLIST && ( -

The list would read:

- )} -
    - {action.checklistItems.map((item, index) => ( -
  • - · {item} -
  • - ))} -
-
+ {action.action === BUDDY_ACTION_AMEND_CHECKLIST && ( +

+ Added to the end of that list — nothing on it changes: +

)} + {action.action === BUDDY_ACTION_TICK_CHECKLIST && ( +

+ Ticked off on that list — nothing else on it changes: +

+ )} + {/* The whole list as it would read, replacing what is there. What would be lost is + named in the preview above, since a missing line is invisible in this one. */} + {action.action === BUDDY_ACTION_EDIT_CHECKLIST && ( +

The list would read:

+ )} +
    + {action.checklistItems.map((item, index) => ( +
  • + · {item} +
  • + ))} +
+
+ )} - {/* The one offer that *replaces* something already on a card, so it shows both: the + {/* The one offer that *replaces* something already on a card, so it shows both: the line as it reads now and as it would read. Only the new wording would be asking them to agree to a change they would have to go and diff for themselves. @@ -181,111 +276,131 @@ export function BuddyActionProposals({ Keyed off the action, like the amend and tick branches above, so a future action that happens to carry both fields does not render as a rewording. */} - {!isStored && - action.action === BUDDY_ACTION_REWORD_CHECKLIST && - action.lineBefore && - action.lineAfter && ( -
- - Currently: - {action.lineBefore} - - - Would become: - {action.lineAfter} - -
- )} + {!isStored && + action.action === BUDDY_ACTION_REWORD_CHECKLIST && + action.lineBefore && + action.lineAfter && ( +
+ + Currently: + {action.lineBefore} + + + Would become: + {action.lineAfter} + +
+ )} - {/* Whitespace kept: the note's first line becomes the card's heading, so a preview that + {/* Whitespace kept: the note's first line becomes the card's heading, so a preview that reflowed it would not be showing what would be kept. */} - {!isStored && action.noteText && ( -

- {action.noteText} -

- )} - {!isStored && } + {!isStored && action.noteText && ( +

+ {action.noteText} +

+ )} + {!isStored && } - {/* A stored proposal warns about itself before it is even clicked: how much it would + {/* A stored proposal warns about itself before it is even clicked: how much it would change decides how loudly the card speaks, before any confirm happens. */} - {isStored && action.risk !== null && } + {isStored && action.risk !== null && } - {isUnsupported && ( - <> -

- This proposal arrived without its details, so it cannot be confirmed here. Ask the - buddy to propose it again. -

-
- -
- - )} - {!isUnsupported && ( -
- {/* `action.label` is written by the model, so its length is not ours to + {isUnsupported && ( + <> +

+ This proposal arrived without its details, so it cannot be confirmed here. Ask the + buddy to propose it again. +

+
+ +
+ + )} + {/* The one proposal whose payload is a *message*: the buddy composes the question, + the hire sends it under their own name, and a person reads it — so it is shown in + full, editable, above the button that sends it. Four things follow from that, and + all are enforced: what goes out is the field's own text (`handleConfirm`), the field + is then made to hold exactly that, an empty field cannot be confirmed and *says why* + (`Field`'s `error` — its presence is the error state, and it is announced), and the + wording is held by the session rather than by this card (`actionDrafts`) — neither a + dock the hire closes nor the hand-off to `/buddy` may throw away a half-worded + question. */} + {isFlagToPm && ( + +