Skip to content

Commit

Permalink
Add confirmation before deleting a hint (Khan#896)
Browse files Browse the repository at this point in the history
## Summary:

Adds a simple confirmation before proceeding with the hint is deleted. This will hopefully prevent alot of frustration by avoiding losing work.

Issue: LC-1591

## Test plan:

Author: jeremywiebe

Reviewers: handeyeco, nixterrimus, jeremywiebe, SonicScrewdriver, nedredmond

Required Reviewers:

Approved By: handeyeco, nixterrimus

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ gerald, ✅ Jest Coverage (ubuntu-latest, 20.x), ⏭  Publish npm snapshot, ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Extract i18n strings (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ gerald, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x)

Pull Request URL: Khan#896
  • Loading branch information
jeremywiebe authored Jan 8, 2024
1 parent 49d5c82 commit 0498106
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/shiny-pugs-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": minor
---

Add a confirmation before deleting a hint in the Exercise Editor
83 changes: 83 additions & 0 deletions packages/perseus-editor/src/__tests__/hint-editor.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import "@testing-library/jest-dom";
import {render, screen} from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import * as React from "react";

import CombinedHintsEditor from "../hint-editor";

describe("CombinedHintsEditor", () => {
it("should render", () => {
render(
<CombinedHintsEditor deviceType="phone" previewURL="about:blank" />,
);
});

it("should confirm before removing a hint", () => {
// Arrange
const comfirmSpy = jest.spyOn(window, "confirm").mockReturnValue(false);
render(
<CombinedHintsEditor
deviceType="phone"
previewURL="about:blank"
hints={[
{content: "You know this one!", widgets: {}, images: {}},
{content: "Ok, the answer is 3", widgets: {}, images: {}},
]}
/>,
);

// Act
userEvent.click(screen.getAllByText("Remove this hint")[0]);

// Assert
expect(comfirmSpy).toHaveBeenCalled();
});

it("should not remove the hint if not confirmed", () => {
// Arrange
jest.spyOn(window, "confirm").mockReturnValue(false);
const onChangeMock = jest.fn();
render(
<CombinedHintsEditor
deviceType="phone"
previewURL="about:blank"
hints={[
{content: "You know this one!", widgets: {}, images: {}},
{content: "Ok, the answer is 3", widgets: {}, images: {}},
]}
onChange={onChangeMock}
/>,
);

// Act
userEvent.click(screen.getAllByText("Remove this hint")[0]);

// Assert
expect(onChangeMock).not.toHaveBeenCalled();
});

it("should remove the hint if confirmed", () => {
// Arrange
jest.spyOn(window, "confirm").mockReturnValue(true);
const onChangeMock = jest.fn();
render(
<CombinedHintsEditor
deviceType="phone"
previewURL="about:blank"
hints={[
{content: "You know this one!", widgets: {}, images: {}},
{content: "Ok, the answer is 3", widgets: {}, images: {}},
]}
onChange={onChangeMock}
/>,
);

// Act
userEvent.click(screen.getAllByText("Remove this hint")[0]);

// Assert
expect(onChangeMock).toHaveBeenCalledWith({
hints: [{content: "Ok, the answer is 3", widgets: {}, images: {}}],
});
});
});
5 changes: 5 additions & 0 deletions packages/perseus-editor/src/hint-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,11 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
};

handleHintRemove: (i: number) => void = (i: number) => {
// eslint-disable-next-line no-alert
if (!confirm("Are you sure you want to delete this hint?")) {
return;
}

const hints = _(this.props.hints).clone();
// @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'.
hints.splice(i, 1);
Expand Down

0 comments on commit 0498106

Please sign in to comment.