From e9951f260b38ee9caec8cf17ef201d93ad08c4ae Mon Sep 17 00:00:00 2001 From: adilallo <39313955+adilallo@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:56:52 -0600 Subject: [PATCH] Let core-value meaning and signals be edited on first open with Add Value, confirm discard when closing without adding, and pin Keep editing to the footer back slot. Co-authored-by: Cursor --- app/(app)/create/CreateFlowLayoutClient.tsx | 19 +- .../methodEditFields/CoreValueEditFields.tsx | 2 +- .../screens/select/CoreValuesSelectScreen.tsx | 279 ++++-------------- .../modals/ModalFooter/ModalFooter.types.ts | 4 + app/components/modals/ModalFooter/index.tsx | 1 + app/hooks/useAsyncConfirm.tsx | 19 +- messages/en/create/customRule/coreValues.json | 7 +- ...unicationMethodsScreenPersistence.test.tsx | 9 +- .../CoreValuesSelectScreen.test.tsx | 41 ++- 9 files changed, 136 insertions(+), 245 deletions(-) diff --git a/app/(app)/create/CreateFlowLayoutClient.tsx b/app/(app)/create/CreateFlowLayoutClient.tsx index ee901b4..7be3c32 100644 --- a/app/(app)/create/CreateFlowLayoutClient.tsx +++ b/app/(app)/create/CreateFlowLayoutClient.tsx @@ -84,6 +84,7 @@ import { SignedInDraftHydration } from "./SignedInDraftHydration"; import { CreateFlowPendingAvatarFlush } from "./components/CreateFlowPendingAvatarFlush"; import Alert from "../../components/modals/Alert"; import Create from "../../components/modals/Create"; +import { MODAL_FOOTER_START_SLOT_CLASS } from "../../components/modals/ModalFooter"; import Share from "../../components/modals/Share"; import { CreateFlowDraftSaveBannerProvider, @@ -637,14 +638,16 @@ function CreateFlowLayoutContent({ nextButtonText={messages.create.topNav.leaveConfirmProceed} onNext={() => closeLeaveConfirm(true)} footerContent={ - +
+ +
} backdropVariant="blurredYellow" ariaLabel={messages.create.topNav.leaveConfirmTitle} diff --git a/app/(app)/create/components/methodEditFields/CoreValueEditFields.tsx b/app/(app)/create/components/methodEditFields/CoreValueEditFields.tsx index defa96d..2f331f1 100644 --- a/app/(app)/create/components/methodEditFields/CoreValueEditFields.tsx +++ b/app/(app)/create/components/methodEditFields/CoreValueEditFields.tsx @@ -15,7 +15,7 @@ import type { CoreValueDetailEntry } from "../../types"; export interface CoreValueEditFieldsProps { value: CoreValueDetailEntry; onChange: (_next: CoreValueDetailEntry) => void; - /** View mode until the user taps **Customize**. */ + /** Disable meaning/signals. Create-flow core-values omits this; final-review locks until Customize. */ readOnly?: boolean; } diff --git a/app/(app)/create/screens/select/CoreValuesSelectScreen.tsx b/app/(app)/create/screens/select/CoreValuesSelectScreen.tsx index cdb4640..9c6ec6e 100644 --- a/app/(app)/create/screens/select/CoreValuesSelectScreen.tsx +++ b/app/(app)/create/screens/select/CoreValuesSelectScreen.tsx @@ -8,7 +8,7 @@ import ContentLockup from "../../../../components/type/ContentLockup"; import { useMessages } from "../../../../contexts/MessagesContext"; import { buildCoreValueChipOptionsFromDraft } from "../../../../../lib/create/coreValueChipOptionsFromDraft"; import { useCreateFlow } from "../../context/CreateFlowContext"; -import { useDiscardCustomizeConfirm } from "../../hooks/useDiscardCustomizeConfirm"; +import { useAsyncConfirm } from "../../../../hooks/useAsyncConfirm"; import type { CommunityStructureChipSnapshotRow, CoreValueDetailEntry, @@ -16,14 +16,7 @@ import type { import { CreateFlowHeaderLockup } from "../../components/CreateFlowHeaderLockup"; import { CreateFlowTwoColumnSelectShell } from "../../components/CreateFlowTwoColumnSelectShell"; import { CoreValueEditFields } from "../../components/methodEditFields"; -import MethodCardCustomizeModalHeader from "../../components/MethodCardCustomizeModalHeader"; import { buildCustomRuleModalKebabMenu } from "../../components/customRuleModalKebabMenu"; -import { - captureMethodCardCustomizeSnapshot, - isMethodCardCustomizeSessionDirty, - type MethodCardCustomizeSnapshot, - type MethodCardHeaderDraft, -} from "../../../../../lib/create/methodCardCustomizeSession"; import { duplicateCoreValueChipInDraft, MAX_SELECTED_CORE_VALUES, @@ -37,8 +30,7 @@ const MAX_CORE_VALUES = MAX_SELECTED_CORE_VALUES; * Why three sessions, not two: * * - `pending` — preset chip just selected; modal opened to capture - * meaning/signals. Dismiss = unselect the chip (keep it in the - * preset row, just not selected). + * meaning/signals. Close (X) confirms, then unselects the chip. * - `customPending` — brand-new custom chip just created via the Add * value flow; modal opened with empty fields. Dismiss = drop the * chip entirely (it was never confirmed via the Add Value button). @@ -101,13 +93,11 @@ export function CoreValuesSelectScreen() { [cv.values], ); - const { confirmDiscard, confirmDirtyCustomizeCancel, confirmDialog } = - useDiscardCustomizeConfirm(); + const { requestConfirm, confirmDialog } = useAsyncConfirm(); const { markCreateFlowInteraction, updateState, replaceState, state } = useCreateFlow(); - const coreCustomizeSnapshotRef = - useRef | null>(null); + const initialDraftRef = useRef(null); const pendingEphemeralCoreDuplicateRef = useRef(null); const [coreValueOptions, setCoreValueOptions] = useState(() => @@ -123,9 +113,6 @@ export function CoreValuesSelectScreen() { ); const [modalSession, setModalSession] = useState(null); const [draft, setDraft] = useState(EMPTY_DETAIL); - const [modalEditUnlocked, setModalEditUnlocked] = useState(false); - const [customizeHeaderDraft, setCustomizeHeaderDraft] = - useState(null); useEffect(() => { setCoreValueOptions( @@ -190,12 +177,11 @@ export function CoreValuesSelectScreen() { valueLabel: string, seedDetail?: CoreValueDetailEntry, ) => { - setDraft(seedDetail ?? getInitialTexts(chipId, valueLabel)); + const initial = seedDetail ?? getInitialTexts(chipId, valueLabel); + initialDraftRef.current = { ...initial }; + setDraft(initial); setActiveModalChipId(chipId); setModalSession(session); - setModalEditUnlocked(false); - setCustomizeHeaderDraft(null); - coreCustomizeSnapshotRef.current = null; markCreateFlowInteraction(); }, [getInitialTexts, markCreateFlowInteraction], @@ -209,87 +195,36 @@ export function CoreValuesSelectScreen() { [markCreateFlowInteraction], ); - const resetCustomizeSession = useCallback(() => { - coreCustomizeSnapshotRef.current = null; - setModalEditUnlocked(false); - setCustomizeHeaderDraft(null); - }, []); - const finalizeModalDismiss = useCallback(() => { pendingEphemeralCoreDuplicateRef.current = null; - resetCustomizeSession(); + initialDraftRef.current = null; setActiveModalChipId(null); setModalSession(null); - }, [resetCustomizeSession]); + }, []); - const handleCustomize = useCallback(() => { - if (!activeModalChipId) return; - const chipLabelNow = - coreValueOptions.find((o) => o.id === activeModalChipId)?.label ?? ""; - if (!chipLabelNow) return; - markCreateFlowInteraction(); - const headerDraft: MethodCardHeaderDraft = { - title: chipLabelNow, - description: "", - }; - coreCustomizeSnapshotRef.current = captureMethodCardCustomizeSnapshot( - draft, - null, - headerDraft, - ); - setCustomizeHeaderDraft(headerDraft); - setModalEditUnlocked(true); - }, [activeModalChipId, coreValueOptions, draft, markCreateFlowInteraction]); - - const handleCancelCustomize = useCallback(async () => { - if (!modalEditUnlocked) return; - const snap = coreCustomizeSnapshotRef.current; - if (!snap) { - resetCustomizeSession(); - return; + const confirmLeaveWithoutSaving = useCallback(async () => { + const isPendingAdd = + modalSession === "pending" || modalSession === "customPending"; + const initial = initialDraftRef.current; + const editingDirty = + modalSession === "editing" && + initial != null && + (draft.meaning !== initial.meaning || draft.signals !== initial.signals); + if (!isPendingAdd && !editingDirty) { + return true; } - if ( - !(await confirmDirtyCustomizeCancel( - snap, - draft, - null, - customizeHeaderDraft, - )) - ) { - return; - } - setDraft(structuredClone(snap.pendingDraft)); - resetCustomizeSession(); - }, [ - confirmDirtyCustomizeCancel, - customizeHeaderDraft, - draft, - modalEditUnlocked, - resetCustomizeSession, - ]); + return requestConfirm({ + title: cv.detailModal.discardTitle, + description: isPendingAdd + ? cv.detailModal.discardPendingDescription + : cv.detailModal.discardEditsDescription, + proceedText: cv.detailModal.discardProceed, + cancelText: cv.detailModal.discardKeepEditing, + }); + }, [cv.detailModal, draft, modalSession, requestConfirm]); - const syncLabelFromCustomizeHeaderToOptions = useCallback(() => { - if (!activeModalChipId || !customizeHeaderDraft) return coreValueOptions; - const trimmed = customizeHeaderDraft.title.trim(); - if (!trimmed) return coreValueOptions; - return coreValueOptions.map((opt) => - opt.id === activeModalChipId ? { ...opt, label: trimmed } : opt, - ); - }, [activeModalChipId, customizeHeaderDraft, coreValueOptions]); - - const handleDuplicateCoreChip = useCallback(async () => { + const handleDuplicateCoreChip = useCallback(() => { if (!activeModalChipId || !modalSession) return; - if ( - !(await confirmDiscard( - modalEditUnlocked, - coreCustomizeSnapshotRef.current, - draft, - null, - customizeHeaderDraft, - )) - ) { - return; - } markCreateFlowInteraction(); const priorEphemeral = pendingEphemeralCoreDuplicateRef.current; let outcome: ReturnType | null = null; @@ -312,7 +247,6 @@ export function CoreValuesSelectScreen() { if (!outcome) { return; } - resetCustomizeSession(); pendingEphemeralCoreDuplicateRef.current = outcome.newId; openModal( outcome.newId, @@ -322,30 +256,15 @@ export function CoreValuesSelectScreen() { ); }, [ activeModalChipId, - confirmDiscard, - customizeHeaderDraft, draft, markCreateFlowInteraction, - modalEditUnlocked, modalKebabMenu.duplicateTitleSuffix, modalSession, openModal, replaceState, - resetCustomizeSession, ]); - const handleRemoveFromKebab = useCallback(async () => { - if ( - !(await confirmDiscard( - modalEditUnlocked, - coreCustomizeSnapshotRef.current, - draft, - null, - customizeHeaderDraft, - )) - ) { - return; - } + const handleRemoveFromKebab = useCallback(() => { markCreateFlowInteraction(); const ep = pendingEphemeralCoreDuplicateRef.current; @@ -386,28 +305,16 @@ export function CoreValuesSelectScreen() { finalizeModalDismiss(); }, [ activeModalChipId, - confirmDiscard, coreValueOptions, - customizeHeaderDraft, - draft, finalizeModalDismiss, markCreateFlowInteraction, - modalEditUnlocked, modalSession, persistCoreValues, replaceState, ]); const handleModalDismiss = useCallback(async () => { - if ( - !(await confirmDiscard( - modalEditUnlocked, - coreCustomizeSnapshotRef.current, - draft, - null, - customizeHeaderDraft, - )) - ) { + if (!(await confirmLeaveWithoutSaving())) { return; } @@ -436,93 +343,39 @@ export function CoreValuesSelectScreen() { finalizeModalDismiss(); }, [ activeModalChipId, - confirmDiscard, + confirmLeaveWithoutSaving, coreValueOptions, - customizeHeaderDraft, - draft, finalizeModalDismiss, - modalEditUnlocked, modalSession, persistCoreValues, replaceState, ]); - const coreCustomizeSaveDisabled = useMemo(() => { - if (!modalEditUnlocked) return false; - const snap = coreCustomizeSnapshotRef.current; - if (!snap) return true; - return !isMethodCardCustomizeSessionDirty( - snap, - draft, - null, - customizeHeaderDraft, - ); - }, [customizeHeaderDraft, draft, modalEditUnlocked]); - const handleModalConfirm = useCallback(() => { if (!activeModalChipId || !modalSession) return; - - if (modalEditUnlocked && customizeHeaderDraft) { - if (coreCustomizeSaveDisabled) { - return; - } - markCreateFlowInteraction(); - pendingEphemeralCoreDuplicateRef.current = null; - const nextOpts = syncLabelFromCustomizeHeaderToOptions(); - persistCoreValues(nextOpts); - updateState({ - coreValueDetailsByChipId: { - ...(state.coreValueDetailsByChipId ?? {}), - [activeModalChipId]: draft, - }, - }); - resetCustomizeSession(); - return; - } - - if (modalSession === "pending" || modalSession === "customPending") { - markCreateFlowInteraction(); - pendingEphemeralCoreDuplicateRef.current = null; - updateState({ - coreValueDetailsByChipId: { - ...(state.coreValueDetailsByChipId ?? {}), - [activeModalChipId]: draft, - }, - }); - resetCustomizeSession(); - setActiveModalChipId(null); - setModalSession(null); - } + markCreateFlowInteraction(); + pendingEphemeralCoreDuplicateRef.current = null; + updateState({ + coreValueDetailsByChipId: { + ...(state.coreValueDetailsByChipId ?? {}), + [activeModalChipId]: draft, + }, + }); + finalizeModalDismiss(); }, [ activeModalChipId, - coreCustomizeSaveDisabled, - customizeHeaderDraft, draft, + finalizeModalDismiss, markCreateFlowInteraction, - modalEditUnlocked, modalSession, - persistCoreValues, - resetCustomizeSession, state.coreValueDetailsByChipId, - syncLabelFromCustomizeHeaderToOptions, updateState, ]); const modalChipLabel = coreValueOptions.find((o) => o.id === activeModalChipId)?.label ?? ""; - const modalFieldsLocked = - !modalEditUnlocked && - Boolean( - modalSession === "pending" || - modalSession === "customPending" || - modalSession === "editing", - ); - - const showFooterPrimary = - modalEditUnlocked || - modalSession === "pending" || - modalSession === "customPending"; + const showFooterPrimary = Boolean(modalSession); const kebabMenuItems = useMemo(() => { if (!modalSession || !activeModalChipId) return []; @@ -530,8 +383,6 @@ export function CoreValuesSelectScreen() { (o) => o.state === "selected", ).length; return buildCustomRuleModalKebabMenu(modalKebabMenu, { - showCustomize: !modalEditUnlocked, - onCustomize: handleCustomize, onDuplicate: modalSession !== "editing" || selectedCount >= MAX_CORE_VALUES ? undefined @@ -542,10 +393,8 @@ export function CoreValuesSelectScreen() { }, [ activeModalChipId, coreValueOptions, - handleCustomize, handleDuplicateCoreChip, handleRemoveFromKebab, - modalEditUnlocked, modalKebabMenu, modalSession, ]); @@ -675,41 +524,22 @@ export function CoreValuesSelectScreen() { onClose={handleModalDismiss} backdropVariant="blurredYellow" headerContent={ - modalEditUnlocked && customizeHeaderDraft ? ( - - setCustomizeHeaderDraft((prev) => - prev ? { ...prev, title } : null, - ) - } - onDescriptionChange={() => {}} - showDescription={false} +
+ - ) : ( -
- -
- ) +
} - showBackButton={modalEditUnlocked} - onBack={handleCancelCustomize} - backButtonText={modalKebabMenu.cancelCustomize} + showBackButton={false} showNextButton={showFooterPrimary} - nextButtonDisabled={ - modalEditUnlocked && coreCustomizeSaveDisabled - } onNext={handleModalConfirm} nextButtonText={ - modalEditUnlocked ? modalKebabMenu.saveEdits : detailModal.addValueButton + modalSession === "editing" + ? modalKebabMenu.saveEdits + : detailModal.addValueButton } kebabTriggerAriaLabel={modalKebabMenu.triggerAriaLabel} kebabMenuAriaLabel={modalKebabMenu.menuAriaLabel} @@ -719,7 +549,6 @@ export function CoreValuesSelectScreen() { ariaLabel={modalChipLabel || "Core value details"} > diff --git a/app/components/modals/ModalFooter/ModalFooter.types.ts b/app/components/modals/ModalFooter/ModalFooter.types.ts index 9f9d305..01fd56f 100644 --- a/app/components/modals/ModalFooter/ModalFooter.types.ts +++ b/app/components/modals/ModalFooter/ModalFooter.types.ts @@ -23,3 +23,7 @@ export interface ModalFooterProps { footerContent?: React.ReactNode; className?: string; } + +/** Left footer slot for confirm-dialog cancel; matches Back (`left` / `top` 16 / 12). */ +export const MODAL_FOOTER_START_SLOT_CLASS = + "absolute left-[16px] top-[12px] flex min-h-[40px] items-center"; diff --git a/app/components/modals/ModalFooter/index.tsx b/app/components/modals/ModalFooter/index.tsx index 0ed8ee8..b8e3341 100644 --- a/app/components/modals/ModalFooter/index.tsx +++ b/app/components/modals/ModalFooter/index.tsx @@ -1,2 +1,3 @@ export { default } from "./ModalFooter.container"; export type { ModalFooterProps } from "./ModalFooter.types"; +export { MODAL_FOOTER_START_SLOT_CLASS } from "./ModalFooter.types"; diff --git a/app/hooks/useAsyncConfirm.tsx b/app/hooks/useAsyncConfirm.tsx index 5f31064..b6652cb 100644 --- a/app/hooks/useAsyncConfirm.tsx +++ b/app/hooks/useAsyncConfirm.tsx @@ -3,6 +3,7 @@ import { useCallback, useRef, useState } from "react"; import Button from "../components/buttons/Button"; import Create from "../components/modals/Create"; +import { MODAL_FOOTER_START_SLOT_CLASS } from "../components/modals/ModalFooter"; import type { CreateModalBackdropVariant } from "../components/modals/Create/CreateModalFrame.view"; export type AsyncConfirmOptions = { @@ -58,14 +59,16 @@ export function useAsyncConfirm() { nextButtonText={options.proceedText} onNext={() => close(true)} footerContent={ - +
+ +
} backdropVariant={options.backdropVariant ?? "blurredYellow"} ariaLabel={options.ariaLabel ?? options.title} diff --git a/messages/en/create/customRule/coreValues.json b/messages/en/create/customRule/coreValues.json index 908bb05..58a3cd9 100644 --- a/messages/en/create/customRule/coreValues.json +++ b/messages/en/create/customRule/coreValues.json @@ -13,7 +13,12 @@ "meaningLabel": "What does this value mean to your group?", "signalsLabel": "Signals of Violation", "addValueButton": "Add Value", - "customizeValueNameLabel": "Value name" + "customizeValueNameLabel": "Value name", + "discardTitle": "Discard unsaved changes?", + "discardPendingDescription": "This value will not be added.", + "discardEditsDescription": "Your edits will not be saved.", + "discardProceed": "Discard", + "discardKeepEditing": "Keep editing" }, "values": [ { diff --git a/tests/components/CommunicationMethodsScreenPersistence.test.tsx b/tests/components/CommunicationMethodsScreenPersistence.test.tsx index c7341e7..7d2f5ba 100644 --- a/tests/components/CommunicationMethodsScreenPersistence.test.tsx +++ b/tests/components/CommunicationMethodsScreenPersistence.test.tsx @@ -263,7 +263,14 @@ describe("CommunicationMethodsScreen — Add Platform persistence", () => { fireEvent.change(textboxes[2], { target: { value: "Edited principle" } }); fireEvent.keyDown(document, { key: "Escape" }); - await screen.findByRole("button", { name: "Keep editing" }); + const keepEditing = await screen.findByRole("button", { + name: "Keep editing", + }); + expect(keepEditing.parentElement).toHaveClass( + "absolute", + "left-[16px]", + "top-[12px]", + ); await declineDiscardCustomizeEdits(); expect(screen.getByRole("dialog")).toBeInTheDocument(); diff --git a/tests/components/CoreValuesSelectScreen.test.tsx b/tests/components/CoreValuesSelectScreen.test.tsx index 37ccd56..007f425 100644 --- a/tests/components/CoreValuesSelectScreen.test.tsx +++ b/tests/components/CoreValuesSelectScreen.test.tsx @@ -9,6 +9,10 @@ describe("CoreValuesSelectScreen", () => { vi.clearAllMocks(); }); + async function discardPendingValue() { + fireEvent.click(await screen.findByRole("button", { name: "Discard" })); + } + it("opens core value detail modal when a preset chip is clicked", async () => { renderWithProviders(); fireEvent.click(screen.getByText("Accessibility")); @@ -16,18 +20,41 @@ describe("CoreValuesSelectScreen", () => { expect( within(dialog).getByRole("button", { name: "Add Value" }), ).toBeInTheDocument(); + expect( + screen.queryByRole("menuitem", { name: "Customize" }), + ).not.toBeInTheDocument(); + fireEvent.click(within(dialog).getByRole("button", { name: "More options" })); + expect( + screen.queryByRole("menuitem", { name: "Customize" }), + ).not.toBeInTheDocument(); }); - it("closes modal and reverts pending selection when Escape is pressed", async () => { + it("asks to discard when closing a pending value, then unselects on Discard", async () => { renderWithProviders(); fireEvent.click(screen.getByText("Accessibility")); await screen.findByRole("dialog"); fireEvent.keyDown(document, { key: "Escape" }); + expect( + await screen.findByRole("button", { name: "Keep editing" }), + ).toBeInTheDocument(); + await discardPendingValue(); await waitFor(() => { expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); }); + it("keeps the pending value modal open when Keep editing is chosen", async () => { + renderWithProviders(); + fireEvent.click(screen.getByText("Accessibility")); + await screen.findByRole("dialog"); + fireEvent.keyDown(document, { key: "Escape" }); + fireEvent.click(await screen.findByRole("button", { name: "Keep editing" })); + const dialog = await screen.findByRole("dialog"); + expect( + within(dialog).getByRole("button", { name: "Add Value" }), + ).toBeInTheDocument(); + }); + it("saves details when Add Value is clicked", async () => { renderWithProviders(); fireEvent.click(screen.getByText("Accessibility")); @@ -40,6 +67,17 @@ describe("CoreValuesSelectScreen", () => { }); }); + it("opens meaning and signals fields editable without Customize", async () => { + renderWithProviders(); + fireEvent.click(screen.getByText("Accessibility")); + const dialog = await screen.findByRole("dialog"); + const fields = within(dialog).getAllByRole("textbox"); + expect(fields.length).toBeGreaterThanOrEqual(2); + for (const field of fields) { + expect(field).toBeEnabled(); + } + }); + // The "Add value" → custom-chip → modal flow uses a `customPending` // session: dismissing the modal must drop the brand-new chip entirely // (not just unselect it), because the user never confirmed it via @@ -80,6 +118,7 @@ describe("CoreValuesSelectScreen", () => { expect(countCustomChips(CUSTOM_LABEL)).toBe(1); fireEvent.keyDown(document, { key: "Escape" }); + fireEvent.click(await screen.findByRole("button", { name: "Discard" })); await waitFor(() => { expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); });