Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions public/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,20 @@
"edit": "Edit",
"view": "View",
"removeStepModal": {
"heading": "Remove instruction step?",
"warning": "Are you sure you want to delete this instructions step? You will not be able to get this back.",
"heading": "Remove instructions?",
"warning": "You will not be able to get deleted instructions back.",
"studentsWarning": "Students who have already started working on this project will still be able to see the instructions as they were when they started.",
"scopeLegend": "What do you want to remove?",
"scope": {
"currentStep": "Remove current step only",
"allSteps": "Remove all steps"
},
"confirm": {
"currentStep": "Remove current step",
"allSteps": "Remove all steps"
},
"cancel": "Cancel",
"removeStep": "Remove step"
"removeInstructions": "Remove instructions"
}
},
"projectsPanel": {
Expand Down
10 changes: 10 additions & 0 deletions src/assets/stylesheets/Instructions.scss
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,13 @@
}
}
}

.modal-content--remove-step {
.modal-content__body {
row-gap: var(--space-2);
}

.rpf-fieldset {
margin-block-end: 0;
}
}
13 changes: 6 additions & 7 deletions src/assets/stylesheets/Modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@
display: flex;
flex-direction: column;

label,
legend {
font-weight: $font-weight-bold;
}

input[type="text"] {
@include font-size-1(regular);
inline-size: 100%;
Expand Down Expand Up @@ -100,6 +95,10 @@
.modal-content__input-section {
display: flex;
flex-direction: column;

label {
font-weight: $font-weight-bold;
}
}

.modal-content__buttons {
Expand Down Expand Up @@ -143,7 +142,7 @@
.modal-overlay {
background-color: rgba(0, 0, 0, 0.5);

input {
input[type="text"] {
border: 2px solid $rpf-white;
background-color: $rpf-grey-700;
color: inherit;
Expand Down Expand Up @@ -175,7 +174,7 @@
.modal-overlay {
background-color: rgba(67, 69, 76, 0.5);

input {
input[type="text"] {
border: 2px solid $rpf-grey-100;

&:focus-visible {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
insertStepAfter,
removeStepAt,
updateStepMarkdown,
REMOVE_ALL_STEPS,
REMOVE_CURRENT_STEP,
} from "../../../../utils/instructionSteps";
import populateMarkdownTemplate from "../../../../utils/populateMarkdownTemplate";
import { Button } from "@raspberrypifoundation/design-system-react";
Expand All @@ -30,6 +32,7 @@ import BinIcon from "../../../../assets/icons/bin.svg";
const InstructionsPanel = () => {
const [tabIndex, setTabIndex] = useState(0);
const [showRemoveStepModal, setShowRemoveStepModal] = useState(false);
const [removeScope, setRemoveScope] = useState(REMOVE_CURRENT_STEP);
const instructionsEditable = useSelector(
(state) => state.editor?.instructionsEditable,
);
Expand Down Expand Up @@ -80,9 +83,25 @@ const InstructionsPanel = () => {
dispatch(setCurrentStepPosition(currentStepPosition + 1));
};

const confirmRemoveStep = () => {
dispatch(setProjectInstructions(removeStepAt(steps, currentStepPosition)));
dispatch(setCurrentStepPosition(Math.max(currentStepPosition - 1, 0)));
const confirmRemoveText = hasMultipleSteps
? t(`instructionsPanel.removeStepModal.confirm.${removeScope}`)
: t("instructionsPanel.removeStepModal.removeInstructions");

const openRemoveStepModal = () => {
setRemoveScope(REMOVE_CURRENT_STEP);
setShowRemoveStepModal(true);
};

const confirmRemove = () => {
if (removeScope === REMOVE_ALL_STEPS) {
dispatch(setProjectInstructions([]));
dispatch(setCurrentStepPosition(0));
} else {
dispatch(
setProjectInstructions(removeStepAt(steps, currentStepPosition)),
);
dispatch(setCurrentStepPosition(Math.max(currentStepPosition - 1, 0)));
}
setShowRemoveStepModal(false);
};

Expand All @@ -97,6 +116,7 @@ const InstructionsPanel = () => {
instructionsEditable && !hasInstructions
? [
<Button
key="add-instructions"
type="primary"
icon={<PlusIcon />}
text={t("instructionsPanel.emptyState.addInstructions")}
Expand Down Expand Up @@ -155,7 +175,7 @@ const InstructionsPanel = () => {
variant="danger"
title={t("instructionsPanel.removeStep")}
icon={<BinIcon />}
onClick={() => setShowRemoveStepModal(true)}
onClick={openRemoveStepModal}
/>
</div>
)}
Expand Down Expand Up @@ -202,8 +222,8 @@ const InstructionsPanel = () => {
type="primary"
key="remove"
variant="danger"
text={t("instructionsPanel.removeStepModal.removeStep")}
onClick={confirmRemoveStep}
text={confirmRemoveText}
onClick={confirmRemove}
/>,
<Button
type="secondary"
Expand All @@ -214,6 +234,9 @@ const InstructionsPanel = () => {
]}
isOpen={showRemoveStepModal}
setShowModal={setShowRemoveStepModal}
showScopeOptions={hasMultipleSteps}
removeScope={removeScope}
setRemoveScope={setRemoveScope}
/>
)}
</SidebarPanel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ const fakeScratchblocksInit = (_locale, container) => {
});
};

const openRemoveModal = () =>
act(() => {
fireEvent.click(screen.getByTitle("instructionsPanel.removeStep"));
});

const removeModalScopeRadio = (scope) =>
screen.getByLabelText(`instructionsPanel.removeStepModal.scope.${scope}`);

const selectRemoveModalScope = (scope) =>
act(() => {
fireEvent.click(removeModalScopeRadio(scope));
});

describe("When instructionsEditable changes from false to true", () => {
test("does not leave the rendered preview above the edit/view tabs", () => {
const { container, store } = renderWithProviders(<InstructionsPanel />, {
Expand Down Expand Up @@ -199,7 +212,9 @@ describe("When instructionsEditable is true", () => {

act(() => {
fireEvent.click(
screen.getByText("instructionsPanel.removeStepModal.removeStep"),
screen.getByText(
"instructionsPanel.removeStepModal.confirm.currentStep",
),
);
});

Expand All @@ -208,6 +223,69 @@ describe("When instructionsEditable is true", () => {
]);
expect(store.getState().instructions.currentStepPosition).toBe(0);
});

test("The confirmation modal offers a scope choice, defaulting to the current step", () => {
openRemoveModal();

expect(removeModalScopeRadio("currentStep")).toBeChecked();
expect(removeModalScopeRadio("allSteps")).not.toBeChecked();
expect(
screen.getByText("instructionsPanel.removeStepModal.studentsWarning"),
).toBeInTheDocument();
});

test("Selecting the remove all steps option relabels the confirm button", () => {
openRemoveModal();
selectRemoveModalScope("allSteps");

expect(removeModalScopeRadio("allSteps")).toBeChecked();
expect(
screen.getByText("instructionsPanel.removeStepModal.confirm.allSteps"),
).toBeInTheDocument();
expect(
screen.queryByText(
"instructionsPanel.removeStepModal.confirm.currentStep",
),
).not.toBeInTheDocument();
});

test("Confirming with the remove all steps option clears the instructions and resets the step position", () => {
act(() => {
store.dispatch(setCurrentStepPosition(1));
});

openRemoveModal();
selectRemoveModalScope("allSteps");

act(() => {
fireEvent.click(
screen.getByText(
"instructionsPanel.removeStepModal.confirm.allSteps",
),
);
});

expect(store.getState().editor.project.instructions).toEqual([]);
expect(store.getState().instructions.currentStepPosition).toBe(0);
expect(
screen.getByText("instructionsPanel.emptyState.addInstructions"),
).toBeInTheDocument();
});

test("Removing the current step is always the default, even after previously choosing to remove all steps", () => {
openRemoveModal();
selectRemoveModalScope("allSteps");

act(() => {
fireEvent.click(
screen.getByText("instructionsPanel.removeStepModal.cancel"),
);
});

openRemoveModal();

expect(removeModalScopeRadio("currentStep")).toBeChecked();
});
});

describe("When there is only one step", () => {
Expand Down Expand Up @@ -241,12 +319,29 @@ describe("When instructionsEditable is true", () => {
).toBeInTheDocument();
});

test("Does not offer the scope choice", () => {
openRemoveModal();

expect(
screen.queryByLabelText(
"instructionsPanel.removeStepModal.scope.currentStep",
),
).not.toBeInTheDocument();
expect(
screen.getByText(
"instructionsPanel.removeStepModal.removeInstructions",
),
).toBeInTheDocument();
});

test("Confirming removal of the only step falls back to the empty state", () => {
fireEvent.click(screen.getByTitle("instructionsPanel.removeStep"));

act(() => {
fireEvent.click(
screen.getByText("instructionsPanel.removeStepModal.removeStep"),
screen.getByText(
"instructionsPanel.removeStepModal.removeInstructions",
),
);
});

Expand Down
3 changes: 2 additions & 1 deletion src/components/Modals/GeneralModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import CloseIcon from "../../utils/CloseIcon";
const GeneralModal = ({
buttons = [],
children,
className = "",
defaultCallback,
heading,
isOpen,
Expand All @@ -30,7 +31,7 @@ const GeneralModal = ({
<Modal
isOpen={isOpen}
onRequestClose={closeModal}
className="modal-content"
className={`modal-content ${className}`.trim()}
overlayClassName="modal-overlay"
contentLabel={heading}
parentSelector={() =>
Expand Down
44 changes: 42 additions & 2 deletions src/components/Modals/RemoveInstructionStepModal.jsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,66 @@
import React from "react";
import { useTranslation } from "react-i18next";
import {
Fieldset,
RadioInput,
} from "@raspberrypifoundation/design-system-react";

import GeneralModal from "./GeneralModal";
import {
REMOVE_ALL_STEPS,
REMOVE_CURRENT_STEP,
} from "../../utils/instructionSteps";

const RemoveInstructionStepModal = (props) => {
const { buttons = null, isOpen = false, setShowModal = null } = props;
const {
buttons = null,
isOpen = false,
setShowModal = null,
showScopeOptions = false,
removeScope = REMOVE_CURRENT_STEP,
setRemoveScope = null,
} = props;

const { t } = useTranslation();

return (
<GeneralModal
className="modal-content--remove-step"
heading={t("instructionsPanel.removeStepModal.heading")}
text={[
{
type: "paragraph",
content: t("instructionsPanel.removeStepModal.warning"),
},
{
type: "paragraph",
content: t("instructionsPanel.removeStepModal.studentsWarning"),
},
]}
buttons={buttons}
isOpen={isOpen}
closeModal={() => setShowModal(false)}
/>
>
{showScopeOptions && (
<Fieldset
legendText={t("instructionsPanel.removeStepModal.scopeLegend")}
fullWidth
>
{[REMOVE_CURRENT_STEP, REMOVE_ALL_STEPS].map((scope) => (
<RadioInput
key={scope}
id={`remove-step-scope-${scope}`}
name="remove-step-scope"
value={scope}
label={t(`instructionsPanel.removeStepModal.scope.${scope}`)}
checked={removeScope === scope}
onChange={() => setRemoveScope(scope)}
fullWidth
/>
))}
</Fieldset>
)}
</GeneralModal>
);
};

Expand Down
3 changes: 3 additions & 0 deletions src/utils/instructionSteps.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export const REMOVE_CURRENT_STEP = "currentStep";
export const REMOVE_ALL_STEPS = "allSteps";

export const insertStepAfter = (steps, index, markdownContent = "") => [
...steps.slice(0, index + 1),
{ markdown_content: markdownContent },
Expand Down
Loading