From 963fa03c5c2cf677966d4fc977933a6a3bba1af8 Mon Sep 17 00:00:00 2001 From: adilallo <39313955+adilallo@users.noreply.github.com> Date: Tue, 25 Aug 2026 09:21:07 -0600 Subject: [PATCH] Close method-card Save like first add, keep key-resource chips on the approach they were chosen for, and leave expanded See-all stacks in catalog order. Co-authored-by: Cursor --- .../components/FinalReviewChipEditModal.tsx | 17 +- .../create/hooks/useMethodCardDeckOrdering.ts | 28 +- .../card/CommunicationMethodsScreen.tsx | 14 +- .../screens/card/ConflictManagementScreen.tsx | 14 +- .../screens/card/MembershipMethodsScreen.tsx | 14 +- .../right-rail/DecisionApproachesScreen.tsx | 134 +++------ lib/create/decisionApproachKeyResources.ts | 28 ++ lib/create/methodCardDisplayOrder.ts | 5 +- lib/create/methodCardSelectionOrder.ts | 4 +- ...unicationMethodsScreenPersistence.test.tsx | 38 +++ tests/pages/decision-approaches.test.jsx | 276 +++++++++++++++++- .../unit/decisionApproachKeyResources.test.ts | 32 ++ 12 files changed, 437 insertions(+), 167 deletions(-) diff --git a/app/(app)/create/components/FinalReviewChipEditModal.tsx b/app/(app)/create/components/FinalReviewChipEditModal.tsx index 4da16bb..266f739 100644 --- a/app/(app)/create/components/FinalReviewChipEditModal.tsx +++ b/app/(app)/create/components/FinalReviewChipEditModal.tsx @@ -35,10 +35,6 @@ import { decisionApproachPresetFor, membershipPresetFor, } from "../../../../lib/create/finalReviewChipPresets"; -import { - applyDecisionApproachKeyResources, - selectedKeyResourceLabelsFromCheckedIds, -} from "../../../../lib/create/decisionApproachKeyResources"; import { isCustomMethodCardId } from "../../../../lib/create/isCustomMethodCardId"; import { usesWizardFieldBlocksModalBody } from "../../../../lib/create/usesWizardFieldBlocksModalBody"; import type { CustomMethodCardFieldBlock } from "../../../../lib/create/customMethodCardFieldBlocks"; @@ -1270,7 +1266,6 @@ function facetSeedSignature( return JSON.stringify({ meta: state.customMethodCardMetaById?.[id] ?? null, details: state.decisionApproachDetailsById?.[id] ?? null, - keyResources: state.selectedDecisionKeyResourceIds ?? null, blocks: state.customMethodCardFieldBlocksById?.[id] ?? null, }); case "conflictManagement": @@ -1335,18 +1330,12 @@ function seedDraftForTarget( const saved = state.decisionApproachDetailsById?.[target.overrideKey] ?? decisionApproachPresetFor(target.overrideKey); - const withKeys = applyDecisionApproachKeyResources( - saved, - selectedKeyResourceLabelsFromCheckedIds( - state.selectedDecisionKeyResourceIds ?? [], - ), - ); return { groupKey: "decisionApproaches", value: { - ...withKeys, - applicableScope: [...withKeys.applicableScope], - selectedApplicableScope: [...withKeys.selectedApplicableScope], + ...saved, + applicableScope: [...saved.applicableScope], + selectedApplicableScope: [...saved.selectedApplicableScope], }, }; } diff --git a/app/(app)/create/hooks/useMethodCardDeckOrdering.ts b/app/(app)/create/hooks/useMethodCardDeckOrdering.ts index 6b0010e..a89b676 100644 --- a/app/(app)/create/hooks/useMethodCardDeckOrdering.ts +++ b/app/(app)/create/hooks/useMethodCardDeckOrdering.ts @@ -1,10 +1,7 @@ "use client"; import { useMemo } from "react"; -import { - mergeCompactCardIdsWithPinnedSelected, - orderRankedMethodsWithPinnedSelection, -} from "../../../../lib/create/methodCardDisplayOrder"; +import { mergeCompactCardIdsWithPinnedSelected } from "../../../../lib/create/methodCardDisplayOrder"; import { deriveCompactCards, rankMethodsByScore, @@ -15,10 +12,10 @@ import { type MethodEntry = { id: string; label: string; supportText: string }; /** - * Applies score ranking, compact-slot rules, then surfaces selected ids first in - * `selected*Ids` order (most-recent add at index 0 via - * {@link moveFacetSelectionIdToFront}). Selection-first applies whenever the facet - * has any selection — not only after footer Confirm (`methodSectionsPinCommitted`). + * Applies score ranking and compact-slot rules. Expanded CardStack order stays + * the ranked catalog (selected cards keep their place). Compact slots still + * pin selected ids first so a pick outside the unpinned top-N remains visible + * when the stack is collapsed. */ export function useMethodCardDeckOrdering( section: RecommendationSection, @@ -35,16 +32,7 @@ export function useMethodCardDeckOrdering( ); const selectionShowcaseActive = selectedIds.length > 0; - - const displayMethods = useMemo( - () => - orderRankedMethodsWithPinnedSelection( - rankedMethods, - selectedIds, - selectionShowcaseActive, - ), - [rankedMethods, selectedIds, selectionShowcaseActive], - ); + const displayMethods = rankedMethods; const { compactCardIds: baseCompactCardIds, recommendedIds } = useMemo( () => @@ -60,13 +48,13 @@ export function useMethodCardDeckOrdering( const compactCardIds = useMemo( () => mergeCompactCardIdsWithPinnedSelected( - displayMethods.map((m) => m.id), + rankedMethods.map((m) => m.id), baseCompactCardIds, selectedIds, selectionShowcaseActive, 5, ), - [displayMethods, baseCompactCardIds, selectedIds, selectionShowcaseActive], + [rankedMethods, baseCompactCardIds, selectedIds, selectionShowcaseActive], ); const sampleCards = useMemo( diff --git a/app/(app)/create/screens/card/CommunicationMethodsScreen.tsx b/app/(app)/create/screens/card/CommunicationMethodsScreen.tsx index 63f7419..e70fd2c 100644 --- a/app/(app)/create/screens/card/CommunicationMethodsScreen.tsx +++ b/app/(app)/create/screens/card/CommunicationMethodsScreen.tsx @@ -643,16 +643,9 @@ export function CommunicationMethodsScreen() { }, }); } - if (pendingDraft) { - customizeSnapshotRef.current = captureMethodCardCustomizeSnapshot( - pendingDraft, - persistWizardBlocks ? draftFieldBlocks : null, - customizeSnapshotRef.current?.headerDraft ?? { - title: "", - description: "", - }, - ); - } + pendingEphemeralDuplicateIdRef.current = null; + customizeSnapshotRef.current = null; + void handleCreateModalClose(); return; } @@ -740,6 +733,7 @@ export function CommunicationMethodsScreen() { title={modalConfig.title} description={modalConfig.description} nextButtonText={modalConfig.nextButtonText} + showBackButton={false} showNextButton={showMethodModalPrimary} backdropVariant="blurredYellow" kebabTriggerAriaLabel={modalKebabMenu.triggerAriaLabel} diff --git a/app/(app)/create/screens/card/ConflictManagementScreen.tsx b/app/(app)/create/screens/card/ConflictManagementScreen.tsx index c469c91..0aa9337 100644 --- a/app/(app)/create/screens/card/ConflictManagementScreen.tsx +++ b/app/(app)/create/screens/card/ConflictManagementScreen.tsx @@ -644,16 +644,9 @@ export function ConflictManagementScreen() { }, }); } - if (pendingDraft) { - customizeSnapshotRef.current = captureMethodCardCustomizeSnapshot( - pendingDraft, - persistWizardBlocks ? draftFieldBlocks : null, - customizeSnapshotRef.current?.headerDraft ?? { - title: "", - description: "", - }, - ); - } + pendingEphemeralDuplicateIdRef.current = null; + customizeSnapshotRef.current = null; + void handleCreateModalClose(); return; } @@ -741,6 +734,7 @@ export function ConflictManagementScreen() { title={modalConfig.title} description={modalConfig.description} nextButtonText={modalConfig.nextButtonText} + showBackButton={false} showNextButton={showMethodModalPrimary} backdropVariant="blurredYellow" kebabTriggerAriaLabel={modalKebabMenu.triggerAriaLabel} diff --git a/app/(app)/create/screens/card/MembershipMethodsScreen.tsx b/app/(app)/create/screens/card/MembershipMethodsScreen.tsx index 187ab40..78532c8 100644 --- a/app/(app)/create/screens/card/MembershipMethodsScreen.tsx +++ b/app/(app)/create/screens/card/MembershipMethodsScreen.tsx @@ -637,16 +637,9 @@ export function MembershipMethodsScreen() { }, }); } - if (pendingDraft) { - customizeSnapshotRef.current = captureMethodCardCustomizeSnapshot( - pendingDraft, - persistWizardBlocks ? draftFieldBlocks : null, - customizeSnapshotRef.current?.headerDraft ?? { - title: "", - description: "", - }, - ); - } + pendingEphemeralDuplicateIdRef.current = null; + customizeSnapshotRef.current = null; + void handleCreateModalClose(); return; } @@ -734,6 +727,7 @@ export function MembershipMethodsScreen() { title={modalConfig.title} description={modalConfig.description} nextButtonText={modalConfig.nextButtonText} + showBackButton={false} showNextButton={showMethodModalPrimary} backdropVariant="blurredYellow" kebabTriggerAriaLabel={modalKebabMenu.triggerAriaLabel} diff --git a/app/(app)/create/screens/right-rail/DecisionApproachesScreen.tsx b/app/(app)/create/screens/right-rail/DecisionApproachesScreen.tsx index 4f43712..adb23da 100644 --- a/app/(app)/create/screens/right-rail/DecisionApproachesScreen.tsx +++ b/app/(app)/create/screens/right-rail/DecisionApproachesScreen.tsx @@ -35,10 +35,8 @@ import { uploadCreateFlowFile } from "../../../../../lib/create/uploadToServer"; import { decisionApproachPresetFor } from "../../../../../lib/create/finalReviewChipPresets"; import { applyDecisionApproachKeyResources, - decisionApproachKeyResourceIdsFromLabels, + decisionApproachKeyResourceCheckboxIds, selectedKeyResourceLabelsFromCheckedIds, - stringArraysEqual, - syncDecisionApproachKeyResourceDetails, } from "../../../../../lib/create/decisionApproachKeyResources"; import type { CustomMethodCardFieldBlock } from "../../../../../lib/create/customMethodCardFieldBlocks"; import { mergePresetMethodsWithCustom } from "../../../../../lib/create/mergePresetMethodsWithCustom"; @@ -94,11 +92,12 @@ export function DecisionApproachesScreen() { >(null); const selectedIds = state.selectedDecisionApproachIds ?? []; - const messageBoxCheckedIds = state.selectedDecisionKeyResourceIds ?? []; - const selectedKeyResourceLabels = useMemo( - () => selectedKeyResourceLabelsFromCheckedIds(messageBoxCheckedIds), - [messageBoxCheckedIds], - ); + const messageBoxCheckedIds = decisionApproachKeyResourceCheckboxIds({ + detailsById: state.decisionApproachDetailsById, + selectedApproachIds: selectedIds, + reminderIds: state.selectedDecisionKeyResourceIds, + openDraft: pendingDraft, + }); const messageBoxItems: InfoMessageBoxItem[] = useMemo( () => @@ -155,23 +154,16 @@ export function DecisionApproachesScreen() { setPendingDraft( applyDecisionApproachKeyResources(pendingDraft, nextLabels), ); + return; } updateState({ selectedDecisionKeyResourceIds: nextCheckedIds, - decisionApproachDetailsById: syncDecisionApproachKeyResourceDetails( - state.decisionApproachDetailsById, - selectedIds, - nextLabels, - decisionApproachPresetFor, - ), }); }, [ markCreateFlowInteraction, messageBoxCheckedIds, pendingDraft, - selectedIds, - state.decisionApproachDetailsById, updateState, ], ); @@ -179,19 +171,12 @@ export function DecisionApproachesScreen() { const seedDraft = useCallback( (id: string): DecisionApproachDetailEntry => { const saved = state.decisionApproachDetailsById?.[id]; - const base = saved - ? { - ...saved, - applicableScope: [...saved.applicableScope], - selectedApplicableScope: [...saved.selectedApplicableScope], - } - : decisionApproachPresetFor(id); - return applyDecisionApproachKeyResources( - base, - selectedKeyResourceLabels, - ); + if (saved) { + return structuredClone(saved); + } + return decisionApproachPresetFor(id); }, - [selectedKeyResourceLabels, state.decisionApproachDetailsById], + [state.decisionApproachDetailsById], ); const handleCardSelect = useCallback( @@ -237,31 +222,8 @@ export function DecisionApproachesScreen() { (next: DecisionApproachDetailEntry) => { markCreateFlowInteraction(); setPendingDraft(next); - const nextCheckedIds = decisionApproachKeyResourceIdsFromLabels( - next.selectedApplicableScope, - ); - if (stringArraysEqual(nextCheckedIds, messageBoxCheckedIds)) { - return; - } - const nextLabels = - selectedKeyResourceLabelsFromCheckedIds(nextCheckedIds); - updateState({ - selectedDecisionKeyResourceIds: nextCheckedIds, - decisionApproachDetailsById: syncDecisionApproachKeyResourceDetails( - state.decisionApproachDetailsById, - selectedIds, - nextLabels, - decisionApproachPresetFor, - ), - }); }, - [ - markCreateFlowInteraction, - messageBoxCheckedIds, - selectedIds, - state.decisionApproachDetailsById, - updateState, - ], + [markCreateFlowInteraction], ); const isSelectedCardModal = @@ -652,10 +614,7 @@ export function DecisionApproachesScreen() { }, decisionApproachDetailsById: { ...(state.decisionApproachDetailsById ?? {}), - [id]: applyDecisionApproachKeyResources( - decisionApproachPresetFor(id), - selectedKeyResourceLabels, - ), + [id]: decisionApproachPresetFor(id), }, customMethodCardFieldBlocksById: { ...(state.customMethodCardFieldBlocksById ?? {}), @@ -667,7 +626,6 @@ export function DecisionApproachesScreen() { markCreateFlowInteraction, pendingDraft, selectedIds, - selectedKeyResourceLabels, state.customMethodCardFieldBlocksById, state.customMethodCardMetaById, state.decisionApproachDetailsById, @@ -687,31 +645,30 @@ export function DecisionApproachesScreen() { modalUsesWizardFieldBlocksBody && draftFieldBlocks !== null; if (selectedIds.includes(pendingCardId)) { - if (persistWizardBlocks) { - updateState({ - customMethodCardFieldBlocksById: { - ...(state.customMethodCardFieldBlocksById ?? {}), - [pendingCardId]: structuredClone(draftFieldBlocks ?? []), - }, - }); - } else if (pendingDraft) { - updateState({ + replaceState((prev) => { + if (persistWizardBlocks) { + return { + ...prev, + customMethodCardFieldBlocksById: { + ...(prev.customMethodCardFieldBlocksById ?? {}), + [pendingCardId]: structuredClone(draftFieldBlocks ?? []), + }, + }; + } + if (!pendingDraft) { + return prev; + } + return { + ...prev, decisionApproachDetailsById: { - ...(state.decisionApproachDetailsById ?? {}), - [pendingCardId]: pendingDraft, + ...(prev.decisionApproachDetailsById ?? {}), + [pendingCardId]: structuredClone(pendingDraft), }, - }); - } - if (pendingDraft) { - customizeSnapshotRef.current = captureMethodCardCustomizeSnapshot( - pendingDraft, - persistWizardBlocks ? draftFieldBlocks : null, - customizeSnapshotRef.current?.headerDraft ?? { - title: "", - description: "", - }, - ); - } + }; + }); + pendingEphemeralDuplicateIdRef.current = null; + customizeSnapshotRef.current = null; + void handleCreateModalClose(); return; } @@ -719,24 +676,25 @@ export function DecisionApproachesScreen() { void handleCreateModalClose(); return; } - updateState({ + replaceState((prev) => ({ + ...prev, selectedDecisionApproachIds: moveFacetSelectionIdToFront( - selectedIds, + prev.selectedDecisionApproachIds ?? [], pendingCardId, ), decisionApproachDetailsById: { - ...(state.decisionApproachDetailsById ?? {}), - [pendingCardId]: pendingDraft, + ...(prev.decisionApproachDetailsById ?? {}), + [pendingCardId]: structuredClone(pendingDraft), }, ...(persistWizardBlocks ? { customMethodCardFieldBlocksById: { - ...(state.customMethodCardFieldBlocksById ?? {}), + ...(prev.customMethodCardFieldBlocksById ?? {}), [pendingCardId]: structuredClone(draftFieldBlocks ?? []), }, } : {}), - }); + })); pendingEphemeralDuplicateIdRef.current = null; customizeSnapshotRef.current = null; void handleCreateModalClose(); @@ -747,9 +705,8 @@ export function DecisionApproachesScreen() { modalUsesWizardFieldBlocksBody, pendingCardId, pendingDraft, + replaceState, selectedIds, - state, - updateState, ]); const modalConfig = pendingCardId @@ -840,6 +797,7 @@ export function DecisionApproachesScreen() { title={modalConfig.title} description={modalConfig.description} nextButtonText={modalConfig.nextButtonText} + showBackButton={false} showNextButton={showMethodModalPrimary} backdropVariant="blurredYellow" kebabTriggerAriaLabel={modalKebabMenu.triggerAriaLabel} diff --git a/lib/create/decisionApproachKeyResources.ts b/lib/create/decisionApproachKeyResources.ts index 5f24705..10ef483 100644 --- a/lib/create/decisionApproachKeyResources.ts +++ b/lib/create/decisionApproachKeyResources.ts @@ -59,6 +59,34 @@ export function decisionApproachKeyResourceIdsFromLabels( .map((item) => item.id); } +/** + * Sidebar checkboxes follow the open approach while its modal is up. + * With no modal they show key resources assigned to any selected approach, + * plus extra reminder checks that are not painted onto chips. + */ +export function decisionApproachKeyResourceCheckboxIds(params: { + detailsById: Record | undefined; + selectedApproachIds: readonly string[]; + reminderIds?: readonly string[]; + openDraft?: DecisionApproachDetailEntry | null; +}): string[] { + if (params.openDraft) { + return decisionApproachKeyResourceIdsFromLabels( + params.openDraft.selectedApplicableScope, + ); + } + const assignedLabels: string[] = []; + for (const id of params.selectedApproachIds) { + const entry = params.detailsById?.[id]; + if (!entry) continue; + assignedLabels.push(...entry.selectedApplicableScope); + } + return uniquePreserveOrder([ + ...decisionApproachKeyResourceIdsFromLabels(assignedLabels), + ...(params.reminderIds ?? []), + ]); +} + export function stringArraysEqual( a: readonly string[], b: readonly string[], diff --git a/lib/create/methodCardDisplayOrder.ts b/lib/create/methodCardDisplayOrder.ts index c9452ee..3bfb2e2 100644 --- a/lib/create/methodCardDisplayOrder.ts +++ b/lib/create/methodCardDisplayOrder.ts @@ -1,7 +1,6 @@ /** - * Reorders facet-ranked method presets so explicitly confirmed selections pin - * to the top while the remainder keeps score-based ranking (recommended before - * default). + * Compact CardStack slots pin confirmed selections to the front while the + * remainder keeps score-based ranking. Expanded order stays the ranked catalog. */ /** Selected ids first (selection array order); then tail in `ranked` order. */ diff --git a/lib/create/methodCardSelectionOrder.ts b/lib/create/methodCardSelectionOrder.ts index 9af1f92..73a46ad 100644 --- a/lib/create/methodCardSelectionOrder.ts +++ b/lib/create/methodCardSelectionOrder.ts @@ -1,7 +1,7 @@ /** * Canonical ordering for method-card facet `selected*Ids` when the user adds a card: - * most recently confirmed id is index 0 so stack / compact layouts stay consistent - * with {@link orderRankedMethodsWithPinnedSelection}. + * most recently confirmed id is index 0 so compact layouts stay consistent + * with {@link mergeCompactCardIdsWithPinnedSelected}. */ export function moveFacetSelectionIdToFront( prev: readonly string[], diff --git a/tests/components/CommunicationMethodsScreenPersistence.test.tsx b/tests/components/CommunicationMethodsScreenPersistence.test.tsx index 9ab2adc..a9da359 100644 --- a/tests/components/CommunicationMethodsScreenPersistence.test.tsx +++ b/tests/components/CommunicationMethodsScreenPersistence.test.tsx @@ -161,6 +161,44 @@ describe("CommunicationMethodsScreen — Add Platform persistence", () => { expect(textareas[2].value).toBe("Saved coc"); }); + it("persists edits and closes when Save is clicked on an already-selected platform", async () => { + let latest: CreateFlowState = {}; + render( + { + latest = s; + }} + initial={{ + selectedCommunicationMethodIds: ["signal"], + communicationMethodDetailsById: { + signal: { + corePrinciple: "Saved principle", + logisticsAdmin: "Saved logistics", + codeOfConduct: "Saved coc", + }, + }, + }} + />, + ); + + fireEvent.click( + screen.getAllByRole("button", { name: /Signal: Encrypted messaging/ })[0], + ); + const dialog = await screen.findByRole("dialog"); + const textareas = within(dialog).getAllByRole( + "textbox", + ) as HTMLTextAreaElement[]; + fireEvent.change(textareas[0], { target: { value: "Edited principle" } }); + fireEvent.click(within(dialog).getByRole("button", { name: "Save" })); + + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + expect( + latest.communicationMethodDetailsById?.signal?.corePrinciple, + ).toBe("Edited principle"); + }); + it("keeps catalog section editors when a title override exists and there are no custom fields", async () => { const details = communicationPresetFor("video-meetings"); const noFieldsHint = diff --git a/tests/pages/decision-approaches.test.jsx b/tests/pages/decision-approaches.test.jsx index d29d0da..73b2901 100644 --- a/tests/pages/decision-approaches.test.jsx +++ b/tests/pages/decision-approaches.test.jsx @@ -3,6 +3,7 @@ import { screen, cleanup, within, + waitFor, } from "../utils/test-utils"; import userEvent from "@testing-library/user-event"; import { describe, test, expect, afterEach } from "vitest"; @@ -161,6 +162,45 @@ describe("Create flow decision-approaches page", () => { ).toBeInTheDocument(); }); + test("selecting an approach in the expanded list does not move it to the top", async () => { + const user = userEvent.setup(); + render(); + + await user.click( + screen.getByRole("button", { name: "See all decision approaches" }), + ); + + const cardLabels = () => + screen + .getAllByRole("button") + .map((el) => el.getAttribute("aria-label") || "") + .filter((label) => label.includes(": ")); + + const labelsBefore = cardLabels(); + expect(labelsBefore[0]).toMatch(/^Lazy Consensus:/); + expect(labelsBefore.some((label) => label.startsWith("Sociocracy:"))).toBe( + true, + ); + + await user.click(screen.getByRole("button", { name: /^Sociocracy:/ })); + await user.click( + within(await screen.findByRole("dialog")).getByRole("button", { + name: "Add Approach", + }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + expect( + screen.getByRole("button", { name: "Show less" }), + ).toBeInTheDocument(); + expect(cardLabels()).toEqual(labelsBefore); + expect( + screen.getByRole("button", { name: /^Sociocracy:/ }), + ).toHaveTextContent("SELECTED"); + }); + test("clicking a card opens the create modal and confirming selects it", async () => { const user = userEvent.setup(); render(); @@ -212,6 +252,33 @@ describe("Create flow decision-approaches page", () => { expect(screen.getByRole("menuitem", { name: "Remove" })).toBeInTheDocument(); }); + test("Save on a selected approach persists the edit and closes", async () => { + const user = userEvent.setup(); + render(); + + const card = screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }); + await user.click(card); + const dialog = await screen.findByRole("dialog"); + await user.click( + within(dialog).getByRole("button", { name: "Add Approach" }), + ); + + await user.click(card); + const dialogAgain = await screen.findByRole("dialog"); + const principleField = within(dialogAgain).getByRole("textbox", { + name: /core principle/i, + }); + await user.clear(principleField); + await user.type(principleField, "Edited principle"); + await user.click(within(dialogAgain).getByRole("button", { name: "Save" })); + + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + }); + test("Remove from the kebab deselects the approach", async () => { const user = userEvent.setup(); render(); @@ -303,7 +370,7 @@ describe("Create flow decision-approaches page", () => { ).toBeInTheDocument(); }); - test("checking a key-resource box selects that chip on every approach", async () => { + test("checking a key-resource box does not highlight that chip on an unselected approach", async () => { const user = userEvent.setup(); render(); @@ -319,10 +386,35 @@ describe("Create flow decision-approaches page", () => { const lazyDialog = await screen.findByRole("dialog"); expect( within(lazyDialog).getByRole("button", { - name: "Deselect Steward finances", + name: "Select Steward finances", }), ).toBeInTheDocument(); - await user.click(within(lazyDialog).getByRole("button", { name: "Close dialog" })); + }); + + test("applicable-scope chips stay on the approach they were chosen for", async () => { + const user = userEvent.setup(); + render(); + + await user.click( + screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }), + ); + const lazyDialog = await screen.findByRole("dialog"); + await user.click( + within(lazyDialog).getByRole("button", { + name: "Select Steward finances", + }), + ); + expect( + screen.getByRole("checkbox", { name: "Steward finances" }), + ).toBeChecked(); + await user.click( + within(lazyDialog).getByRole("button", { name: "Add Approach" }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); await user.click( screen.getByRole("button", { @@ -332,12 +424,45 @@ describe("Create flow decision-approaches page", () => { const doocracyDialog = await screen.findByRole("dialog"); expect( within(doocracyDialog).getByRole("button", { + name: "Select Steward finances", + }), + ).toBeInTheDocument(); + expect( + within(doocracyDialog).queryByRole("button", { + name: "Deselect Steward finances", + }), + ).not.toBeInTheDocument(); + expect( + screen.getByRole("checkbox", { name: "Steward finances" }), + ).not.toBeChecked(); + await user.click( + within(doocracyDialog).getByRole("button", { name: "Close dialog" }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + expect( + screen.getByRole("checkbox", { name: "Steward finances" }), + ).toBeChecked(); + + await user.click( + screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }), + ); + const lazyAgain = await screen.findByRole("dialog"); + expect( + within(lazyAgain).getByRole("button", { name: "Deselect Steward finances", }), ).toBeInTheDocument(); + expect( + screen.getByRole("checkbox", { name: "Steward finances" }), + ).toBeChecked(); }); - test("choosing a key-resource chip checks the matching sidebar box", async () => { + test("checking a key-resource box with a modal open selects that chip on the open approach only", async () => { const user = userEvent.setup(); render(); @@ -346,17 +471,148 @@ describe("Create flow decision-approaches page", () => { name: /Lazy Consensus: A decision is assumed approved/, }), ); - const dialog = await screen.findByRole("dialog"); + const lazyDialog = await screen.findByRole("dialog"); await user.click( - within(dialog).getByRole("button", { - name: "Select Discipline and member termination", + within(lazyDialog).getByRole("button", { name: "Add Approach" }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + await user.click( + screen.getByRole("button", { + name: /Do-ocracy: Decisions are made by those who take initiative/, }), ); + const doocracyDialog = await screen.findByRole("dialog"); + await user.click( + screen.getByRole("checkbox", { name: "Steward finances" }), + ); + expect( + within(doocracyDialog).getByRole("button", { + name: "Deselect Steward finances", + }), + ).toBeInTheDocument(); + await user.click( + within(doocracyDialog).getByRole("button", { name: "Close dialog" }), + ); + await user.click(await screen.findByRole("button", { name: "Discard" })); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + await user.click( + screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }), + ); + const lazyAgain = await screen.findByRole("dialog"); + expect( + within(lazyAgain).getByRole("button", { + name: "Select Steward finances", + }), + ).toBeInTheDocument(); + }); + + test("highlighting a key-resource chip on a second approach does not rewrite the first", async () => { + const user = userEvent.setup(); + render(); + + await user.click( + screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }), + ); + const lazyDialog = await screen.findByRole("dialog"); + await user.click( + within(lazyDialog).getByRole("button", { + name: "Select Steward finances", + }), + ); + expect( + screen.getByRole("checkbox", { name: "Steward finances" }), + ).toBeChecked(); + await user.click( + within(lazyDialog).getByRole("button", { name: "Add Approach" }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + await user.click( + screen.getByRole("button", { + name: /Do-ocracy: Decisions are made by those who take initiative/, + }), + ); + const doocracyDialog = await screen.findByRole("dialog"); + await user.click( + within(doocracyDialog).getByRole("button", { + name: "Select Project level decisions", + }), + ); + expect( + screen.getByRole("checkbox", { name: "Project level decisions" }), + ).toBeChecked(); + expect( + screen.getByRole("checkbox", { name: "Steward finances" }), + ).not.toBeChecked(); + expect( + within(doocracyDialog).getByRole("button", { + name: "Select Steward finances", + }), + ).toBeInTheDocument(); + await user.click( + within(doocracyDialog).getByRole("button", { name: "Add Approach" }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); expect( - screen.getByRole("checkbox", { - name: "Discipline and member termination", - }), + screen.getByRole("checkbox", { name: "Steward finances" }), ).toBeChecked(); + expect( + screen.getByRole("checkbox", { name: "Project level decisions" }), + ).toBeChecked(); + + await user.click( + screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }), + ); + const lazyAgain = await screen.findByRole("dialog"); + expect( + within(lazyAgain).getByRole("button", { + name: "Deselect Steward finances", + }), + ).toBeInTheDocument(); + expect( + within(lazyAgain).getByRole("button", { + name: "Select Project level decisions", + }), + ).toBeInTheDocument(); + await user.click( + within(lazyAgain).getByRole("button", { name: "Close dialog" }), + ); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + await user.click( + screen.getByRole("button", { + name: /Do-ocracy: Decisions are made by those who take initiative/, + }), + ); + const doocracyAgain = await screen.findByRole("dialog"); + expect( + within(doocracyAgain).getByRole("button", { + name: "Deselect Project level decisions", + }), + ).toBeInTheDocument(); + expect( + within(doocracyAgain).getByRole("button", { + name: "Select Steward finances", + }), + ).toBeInTheDocument(); }); }); diff --git a/tests/unit/decisionApproachKeyResources.test.ts b/tests/unit/decisionApproachKeyResources.test.ts index 5b24ea2..c9beb7a 100644 --- a/tests/unit/decisionApproachKeyResources.test.ts +++ b/tests/unit/decisionApproachKeyResources.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { applyDecisionApproachKeyResources, + decisionApproachKeyResourceCheckboxIds, decisionApproachKeyResourceIdsFromLabels, decisionApproachScopeForPublish, decisionApproachKeyResourceItems, @@ -96,6 +97,37 @@ describe("decisionApproachKeyResources", () => { ).toEqual(["amend", "project"]); }); + it("derives sidebar checks from the open draft or selected approaches", () => { + const detailsById = { + "lazy-consensus": { + ...emptyEntry(), + selectedApplicableScope: ["Steward finances"], + }, + "do-ocracy": { + ...emptyEntry(), + selectedApplicableScope: ["Project level decisions"], + }, + }; + expect( + decisionApproachKeyResourceCheckboxIds({ + detailsById, + selectedApproachIds: ["lazy-consensus", "do-ocracy"], + reminderIds: ["discipline"], + }), + ).toEqual(["finances", "project", "discipline"]); + expect( + decisionApproachKeyResourceCheckboxIds({ + detailsById, + selectedApproachIds: ["lazy-consensus", "do-ocracy"], + reminderIds: ["discipline"], + openDraft: { + ...emptyEntry(), + selectedApplicableScope: ["Amend your CommunityRule"], + }, + }), + ).toEqual(["amend"]); + }); + it("syncs existing and selected approach details", () => { const next = syncDecisionApproachKeyResourceDetails( { "lazy-consensus": emptyEntry() },