From 84ef943df44ba0fe3444c1bcd16da95272a2e012 Mon Sep 17 00:00:00 2001 From: adilallo <39313955+adilallo@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:45:56 -0600 Subject: [PATCH] Show a footer Remove on selected create-flow modules so removal is not kebab-only. Co-authored-by: Cursor --- .../components/FinalReviewChipEditModal.tsx | 14 +++++++- .../card/CommunicationMethodsScreen.tsx | 10 +++--- .../screens/card/ConflictManagementScreen.tsx | 2 ++ .../screens/card/MembershipMethodsScreen.tsx | 2 ++ .../screens/review/FinalReviewScreen.tsx | 5 +-- .../right-rail/DecisionApproachesScreen.tsx | 2 ++ .../screens/select/CoreValuesSelectScreen.tsx | 2 ++ .../modals/Create/Create.container.tsx | 6 ++++ app/components/modals/Create/Create.types.ts | 6 ++++ app/components/modals/Create/Create.view.tsx | 6 ++++ .../ModalFooter/ModalFooter.container.tsx | 2 ++ .../modals/ModalFooter/ModalFooter.types.ts | 10 ++++++ .../modals/ModalFooter/ModalFooter.view.tsx | 19 +++++++++- messages/en/common.json | 1 + stories/modals/Create.stories.js | 25 +++++++++++++ stories/modals/ModalFooter.stories.js | 15 ++++++++ .../ConflictManagementScreen.test.tsx | 3 ++ .../CoreValuesSelectScreen.test.tsx | 6 ++++ tests/components/Create.test.tsx | 35 +++++++++++++++++++ tests/components/FinalReviewPage.test.tsx | 33 +++++++++++++++-- .../MembershipMethodsScreen.test.tsx | 3 ++ tests/pages/communication-methods.test.jsx | 30 ++++++++++++++-- tests/pages/decision-approaches.test.jsx | 30 ++++++++++++++-- 23 files changed, 251 insertions(+), 16 deletions(-) diff --git a/app/(app)/create/components/FinalReviewChipEditModal.tsx b/app/(app)/create/components/FinalReviewChipEditModal.tsx index 99d3d90..1d71585 100644 --- a/app/(app)/create/components/FinalReviewChipEditModal.tsx +++ b/app/(app)/create/components/FinalReviewChipEditModal.tsx @@ -4,7 +4,8 @@ * Final-review chip modal: **Core values** and **method** facets share the * kebab → **Duplicate** (values only when under the cap) / **Remove** pattern * from the create-card facet modals (`Create` + - * {@link buildCustomRuleModalKebabMenu}). Values and method chips also offer + * {@link buildCustomRuleModalKebabMenu}), plus a footer **Remove** when the + * chip is already in the selection. Values and method chips also offer * **Customize**, which opens {@link CustomMethodCardWizard} prefilled from the * chip. Fields are editable on open; Save persists body edits without renaming. * @@ -1073,12 +1074,23 @@ export function FinalReviewChipEditModal({ ); }, [subtitle, target]); + const showFooterRemove = + target != null && + (target.groupKey === "coreValues" || isChipInSelection); + return ( <> ( footerClassName, showBackButton = true, showNextButton = true, + showRemoveButton = false, onBack, onNext, + onRemove, backButtonText = "Back", nextButtonText = "Next", + removeButtonText, nextButtonDisabled = false, currentStep, totalSteps, @@ -56,10 +59,13 @@ const CreateContainer = memo( footerClassName={footerClassName} showBackButton={showBackButton} showNextButton={showNextButton} + showRemoveButton={showRemoveButton} onBack={onBack} onNext={onNext} + onRemove={onRemove} backButtonText={backButtonText} nextButtonText={nextButtonText} + removeButtonText={removeButtonText} nextButtonDisabled={nextButtonDisabled} currentStep={currentStep} totalSteps={totalSteps} diff --git a/app/components/modals/Create/Create.types.ts b/app/components/modals/Create/Create.types.ts index 7dcb769..b15f4ca 100644 --- a/app/components/modals/Create/Create.types.ts +++ b/app/components/modals/Create/Create.types.ts @@ -16,10 +16,13 @@ export interface CreateProps { footerClassName?: string; showBackButton?: boolean; showNextButton?: boolean; + showRemoveButton?: boolean; onBack?: () => void; onNext?: () => void; + onRemove?: () => void; backButtonText?: string; nextButtonText?: string; + removeButtonText?: string; nextButtonDisabled?: boolean; currentStep?: number; totalSteps?: number; @@ -58,10 +61,13 @@ export interface CreateViewProps { footerClassName?: string; showBackButton: boolean; showNextButton: boolean; + showRemoveButton: boolean; onBack?: () => void; onNext?: () => void; + onRemove?: () => void; backButtonText: string; nextButtonText: string; + removeButtonText?: string; nextButtonDisabled: boolean; currentStep?: number; totalSteps?: number; diff --git a/app/components/modals/Create/Create.view.tsx b/app/components/modals/Create/Create.view.tsx index 7db1283..8e21bdf 100644 --- a/app/components/modals/Create/Create.view.tsx +++ b/app/components/modals/Create/Create.view.tsx @@ -17,10 +17,13 @@ export function CreateView({ footerClassName, showBackButton, showNextButton, + showRemoveButton, onBack, onNext, + onRemove, backButtonText, nextButtonText, + removeButtonText, nextButtonDisabled, currentStep, totalSteps, @@ -76,10 +79,13 @@ export function CreateView({ ((props) => { const t = useTranslation("common"); const resolvedBackText = props.backButtonText ?? t("buttons.back"); const resolvedNextText = props.nextButtonText ?? t("buttons.next"); + const resolvedRemoveText = props.removeButtonText ?? t("buttons.remove"); return ( ); }); diff --git a/app/components/modals/ModalFooter/ModalFooter.types.ts b/app/components/modals/ModalFooter/ModalFooter.types.ts index 01fd56f..c7b439f 100644 --- a/app/components/modals/ModalFooter/ModalFooter.types.ts +++ b/app/components/modals/ModalFooter/ModalFooter.types.ts @@ -1,8 +1,14 @@ export interface ModalFooterProps { showBackButton?: boolean; showNextButton?: boolean; + /** + * Danger **Remove** in the left footer slot (selected create-flow modules). + * Takes the left slot instead of Back when both would otherwise show. + */ + showRemoveButton?: boolean; onBack?: () => void; onNext?: () => void; + onRemove?: () => void; /** * Custom back button text. If not provided, uses localized "Back" from common.json */ @@ -11,6 +17,10 @@ export interface ModalFooterProps { * Custom next button text. If not provided, uses localized "Next" from common.json */ nextButtonText?: string; + /** + * Custom remove button text. If not provided, uses localized "Remove" from common.json + */ + removeButtonText?: string; nextButtonDisabled?: boolean; currentStep?: number; totalSteps?: number; diff --git a/app/components/modals/ModalFooter/ModalFooter.view.tsx b/app/components/modals/ModalFooter/ModalFooter.view.tsx index 43b7134..b71781a 100644 --- a/app/components/modals/ModalFooter/ModalFooter.view.tsx +++ b/app/components/modals/ModalFooter/ModalFooter.view.tsx @@ -7,10 +7,13 @@ import type { ModalFooterProps } from "./ModalFooter.types"; export function ModalFooterView({ showBackButton = false, showNextButton = false, + showRemoveButton = false, onBack, onNext, + onRemove, backButtonText, nextButtonText, + removeButtonText, nextButtonDisabled = false, currentStep, totalSteps, @@ -22,12 +25,26 @@ export function ModalFooterView({ stepperProp !== undefined ? stepperProp : currentStep !== undefined && totalSteps !== undefined; + const showStartBack = showBackButton && !showRemoveButton; return (
- {showBackButton && ( + {showRemoveButton && ( +
+ +
+ )} + + {showStartBack && (
+ ), + showBackButton: false, + showRemoveButton: true, + showNextButton: true, + nextButtonText: "Save", + nextButtonDisabled: false, + }, + render: Template, +}; export const NextButtonDisabled = { args: { isOpen: true, diff --git a/stories/modals/ModalFooter.stories.js b/stories/modals/ModalFooter.stories.js index 09b53ad..9d380e5 100644 --- a/stories/modals/ModalFooter.stories.js +++ b/stories/modals/ModalFooter.stories.js @@ -11,6 +11,11 @@ export default { control: "boolean", description: "Whether to render the back button on the left", }, + showRemoveButton: { + control: "boolean", + description: + "Whether to render a danger Remove button in the left footer slot", + }, showNextButton: { control: "boolean", description: "Whether to render the next button on the right", @@ -41,6 +46,7 @@ export default { }, onBack: { action: "back-clicked" }, onNext: { action: "next-clicked" }, + onRemove: { action: "remove-clicked" }, }, }; @@ -69,3 +75,12 @@ export const NextOnly = { showNextButton: true, }, }; + +export const RemoveAndSave = { + args: { + showBackButton: false, + showRemoveButton: true, + showNextButton: true, + nextButtonText: "Save", + }, +}; diff --git a/tests/components/ConflictManagementScreen.test.tsx b/tests/components/ConflictManagementScreen.test.tsx index 2d2a68b..c9e4a15 100644 --- a/tests/components/ConflictManagementScreen.test.tsx +++ b/tests/components/ConflictManagementScreen.test.tsx @@ -27,6 +27,9 @@ describe("ConflictManagementScreen", () => { for (const field of fields) { expect(field).toBeEnabled(); } + expect( + within(dialog).queryByRole("button", { name: "Remove" }), + ).not.toBeInTheDocument(); fireEvent.click( within(dialog).getByRole("button", { name: "More options" }), ); diff --git a/tests/components/CoreValuesSelectScreen.test.tsx b/tests/components/CoreValuesSelectScreen.test.tsx index 05b409e..b36eda6 100644 --- a/tests/components/CoreValuesSelectScreen.test.tsx +++ b/tests/components/CoreValuesSelectScreen.test.tsx @@ -26,6 +26,9 @@ describe("CoreValuesSelectScreen", () => { expect( within(dialog).getByRole("button", { name: "Add Value" }), ).toBeInTheDocument(); + expect( + within(dialog).queryByRole("button", { name: "Remove" }), + ).not.toBeInTheDocument(); fireEvent.click(within(dialog).getByRole("button", { name: "More options" })); expect( screen.getByRole("menuitem", { name: "Customize" }), @@ -124,6 +127,9 @@ describe("CoreValuesSelectScreen", () => { }); fireEvent.click(screen.getByText("Accessibility")); const editing = await screen.findByRole("dialog"); + expect( + within(editing).getByRole("button", { name: "Remove" }), + ).toBeInTheDocument(); fireEvent.click( within(editing).getByRole("button", { name: "More options" }), ); diff --git a/tests/components/Create.test.tsx b/tests/components/Create.test.tsx index d6d6d14..75689ab 100644 --- a/tests/components/Create.test.tsx +++ b/tests/components/Create.test.tsx @@ -169,6 +169,41 @@ describe("Create", () => { expect(screen.getByText("Custom Footer")).toBeInTheDocument(); }); + it("renders a danger Remove in the left footer when showRemoveButton is true", () => { + const onRemove = vi.fn(); + renderWithProviders( + , + ); + const removeButton = screen.getByRole("button", { name: "Remove" }); + expect(removeButton).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Back" })).not.toBeInTheDocument(); + fireEvent.click(removeButton); + expect(onRemove).toHaveBeenCalledTimes(1); + }); + + it("prefers Remove over Back when both would occupy the left slot", () => { + renderWithProviders( + , + ); + expect(screen.getByRole("button", { name: "Remove" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Back" })).not.toBeInTheDocument(); + }); + it("uses responsive width at baseline (matches Login modal)", () => { renderWithProviders( Create dialog content, diff --git a/tests/components/FinalReviewPage.test.tsx b/tests/components/FinalReviewPage.test.tsx index 77d818f..178e3f8 100644 --- a/tests/components/FinalReviewPage.test.tsx +++ b/tests/components/FinalReviewPage.test.tsx @@ -519,7 +519,7 @@ describe("FinalReviewScreen — chip detail modal", () => { ).not.toBeInTheDocument(); }); - it("closes the chip edit modal when Back is pressed", async () => { + it("closes the chip edit modal when the close control is pressed", async () => { render( {}} @@ -532,12 +532,41 @@ describe("FinalReviewScreen — chip detail modal", () => { fireEvent.click(await screen.findByRole("button", { name: "Signal" })); const dialog = await screen.findByRole("dialog"); - fireEvent.click(within(dialog).getByRole("button", { name: "Back" })); + expect( + within(dialog).getByRole("button", { name: "Remove" }), + ).toBeInTheDocument(); + expect( + within(dialog).queryByRole("button", { name: "Back" }), + ).not.toBeInTheDocument(); + fireEvent.click(within(dialog).getByLabelText("Close dialog")); await waitFor(() => { expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); }); + it("deselects a method chip from the footer Remove", async () => { + let latest: CreateFlowState = {}; + render( + { + latest = s; + }} + initial={{ + title: "Oak Park Commons", + selectedCommunicationMethodIds: ["signal"], + }} + />, + ); + + fireEvent.click(await screen.findByRole("button", { name: "Signal" })); + const dialog = await screen.findByRole("dialog"); + fireEvent.click(within(dialog).getByRole("button", { name: "Remove" })); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + expect(latest.selectedCommunicationMethodIds ?? []).not.toContain("signal"); + }); + }); /** diff --git a/tests/components/MembershipMethodsScreen.test.tsx b/tests/components/MembershipMethodsScreen.test.tsx index c10e7c7..7f3791c 100644 --- a/tests/components/MembershipMethodsScreen.test.tsx +++ b/tests/components/MembershipMethodsScreen.test.tsx @@ -27,6 +27,9 @@ describe("MembershipMethodsScreen", () => { for (const field of fields) { expect(field).toBeEnabled(); } + expect( + within(dialog).queryByRole("button", { name: "Remove" }), + ).not.toBeInTheDocument(); fireEvent.click( within(dialog).getByRole("button", { name: "More options" }), ); diff --git a/tests/pages/communication-methods.test.jsx b/tests/pages/communication-methods.test.jsx index 2993869..b639036 100644 --- a/tests/pages/communication-methods.test.jsx +++ b/tests/pages/communication-methods.test.jsx @@ -40,7 +40,7 @@ describe("Create flow communication-methods page", () => { expect(within(dialog).getByText("Add Platform")).toBeInTheDocument(); }); - test("re-opening a selected method shows Save; Remove is in the kebab", async () => { + test("re-opening a selected method shows Save and footer Remove; Remove stays in the kebab", async () => { const user = userEvent.setup(); render(); @@ -54,8 +54,8 @@ describe("Create flow communication-methods page", () => { await user.click(signalCards[0]); const dialogAgain = screen.getByRole("dialog"); expect( - within(dialogAgain).queryByRole("button", { name: "Remove" }), - ).not.toBeInTheDocument(); + within(dialogAgain).getByRole("button", { name: "Remove" }), + ).toBeInTheDocument(); expect( within(dialogAgain).queryByRole("button", { name: "Add Platform" }), ).not.toBeInTheDocument(); @@ -67,6 +67,30 @@ describe("Create flow communication-methods page", () => { expect(screen.getByRole("menuitem", { name: "Remove" })).toBeInTheDocument(); }); + test("Remove from the footer deselects the method", async () => { + const user = userEvent.setup(); + render(); + + const signalCards = screen.getAllByRole("button", { + name: /Signal: Encrypted messaging/, + }); + await user.click(signalCards[0]); + await user.click( + within(screen.getByRole("dialog")).getByRole("button", { + name: "Add Platform", + }), + ); + + expect(signalCards[0]).toHaveTextContent("SELECTED"); + + await user.click(signalCards[0]); + await user.click( + within(screen.getByRole("dialog")).getByRole("button", { name: "Remove" }), + ); + + expect(signalCards[0]).not.toHaveTextContent("SELECTED"); + }); + test("Remove from the kebab deselects the method", async () => { const user = userEvent.setup(); render(); diff --git a/tests/pages/decision-approaches.test.jsx b/tests/pages/decision-approaches.test.jsx index 701d8e3..cf74016 100644 --- a/tests/pages/decision-approaches.test.jsx +++ b/tests/pages/decision-approaches.test.jsx @@ -244,7 +244,7 @@ describe("Create flow decision-approaches page", () => { expect(screen.getByText("SELECTED")).toBeInTheDocument(); }); - test("re-opening a selected approach shows Save; Remove is in the kebab", async () => { + test("re-opening a selected approach shows Save and footer Remove; Remove stays in the kebab", async () => { const user = userEvent.setup(); render(); @@ -262,8 +262,8 @@ describe("Create flow decision-approaches page", () => { await user.click(card); const dialogAgain = screen.getByRole("dialog"); expect( - within(dialogAgain).queryByRole("button", { name: "Remove" }), - ).not.toBeInTheDocument(); + within(dialogAgain).getByRole("button", { name: "Remove" }), + ).toBeInTheDocument(); expect( within(dialogAgain).queryByRole("button", { name: "Add Approach" }), ).not.toBeInTheDocument(); @@ -275,6 +275,30 @@ describe("Create flow decision-approaches page", () => { expect(screen.getByRole("menuitem", { name: "Remove" })).toBeInTheDocument(); }); + test("Remove from the footer deselects the approach", async () => { + const user = userEvent.setup(); + render(); + + const card = screen.getByRole("button", { + name: /Lazy Consensus: A decision is assumed approved/, + }); + await user.click(card); + await user.click( + within(await screen.findByRole("dialog")).getByRole("button", { + name: "Add Approach", + }), + ); + + expect(card).toHaveTextContent("SELECTED"); + + await user.click(card); + await user.click( + within(screen.getByRole("dialog")).getByRole("button", { name: "Remove" }), + ); + + expect(card).not.toHaveTextContent("SELECTED"); + }); + test("Save on a selected approach persists the edit and closes", async () => { const user = userEvent.setup(); render();