Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default to onDiscard in UnsavedChangesModal #21756

Merged
Merged
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
12 changes: 6 additions & 6 deletions packages/js/src/shared-admin/components/unsaved-changes-modal.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
import { __ } from "@wordpress/i18n";
import { Modal, Button, useSvgAria } from "@yoast/ui-library";
import { ExclamationIcon } from "@heroicons/react/outline";
import { __ } from "@wordpress/i18n";
import { Button, Modal, useSvgAria } from "@yoast/ui-library";
import { noop } from "lodash";
import PropTypes from "prop-types";

/**
* The unsaved changes modal.
*
* @param {boolean} isOpen Whether the modal is open.
* @param {Function} onClose The function to call when the modal is closed.
* @param {function} [onClose] The function to call when the modal is closed.
* @param {function} [onDiscard] The function to call when the changes are discarded.
* @param {string} title The title of the modal.
* @param {string} description The description of the modal.
* @param {Function} onDiscard The function to call when the changes are discarded.
* @param {string} dismissLabel The label for the dismiss button.
* @param {string} discardLabel The label for the discard button.
*
* @returns {JSX.Element} The unsaved changes modal.
*/
export const UnsavedChangesModal = ( { isOpen, onClose = noop, title, description, onDiscard, dismissLabel, discardLabel } ) => {
export const UnsavedChangesModal = ( { isOpen, onClose = noop, onDiscard = noop, title, description, dismissLabel, discardLabel } ) => {
const svgAriaProps = useSvgAria();

return <Modal isOpen={ isOpen } onClose={ onClose }>
Expand Down Expand Up @@ -52,9 +52,9 @@ export const UnsavedChangesModal = ( { isOpen, onClose = noop, title, descriptio
UnsavedChangesModal.propTypes = {
isOpen: PropTypes.bool.isRequired,
onClose: PropTypes.func,
onDiscard: PropTypes.func,
title: PropTypes.string.isRequired,
description: PropTypes.string.isRequired,
onDiscard: PropTypes.func.isRequired,
dismissLabel: PropTypes.string.isRequired,
discardLabel: PropTypes.string.isRequired,
};
Original file line number Diff line number Diff line change
@@ -1,36 +1,91 @@
import { render, screen } from "../../test-utils";
import { beforeEach, describe, expect, it, jest } from "@jest/globals";
import { UnsavedChangesModal } from "../../../src/shared-admin/components";
import { fireEvent, render, screen } from "../../test-utils";

describe( "UnsavedChangesModal", () => {
const onClose = jest.fn();
const onDiscard = jest.fn();

beforeEach( () => {
jest.clearAllMocks();
render( <UnsavedChangesModal
isOpen={ true }
onClose={ jest.fn() }
onClose={ onClose }
onDiscard={ onDiscard }
title="Unsaved changes"
description="There are unsaved changes in one or more steps of the first-time configuration. Leaving means that those changes will be lost. Are you sure you want to leave this page?"
onDiscard={ jest.fn() }
dismissLabel="No, continue editing"
discardLabel="Yes, leave page"
/> );
} );

it( "should have dismiss and leave page buttons", () => {
const leaveButton = screen.queryByText( "Yes, leave page" );
expect( leaveButton ).toBeInTheDocument();
it( "should have a role of dialog", () => {
expect( screen.queryByRole( "dialog" ) ).toBeInTheDocument();
} );

expect( screen.queryByText( "Close" ) ).toBeInTheDocument();
it.each( [
[ "dismiss", "No, continue editing" ],
[ "discard", "Yes, leave page" ],
] )( "should have the %s button: %s", ( _, text ) => {
const button = screen.queryByText( text );
expect( button ).toBeInTheDocument();
expect( button.tagName ).toBe( "BUTTON" );
expect( button ).toHaveAttribute( "type", "button" );
} );

const dismissButton = screen.queryByText( "No, continue editing" );
expect( dismissButton ).toBeInTheDocument();
it( "should have the close button", () => {
const screenReaderText = screen.queryByText( "Close" );
expect( screenReaderText ).toBeInTheDocument();
expect( screenReaderText.parentElement ).toHaveAttribute( "type", "button" );
expect( screenReaderText.parentElement.tagName ).toBe( "BUTTON" );
} );

it( "should have a title", () => {
const title = screen.queryByText( "Unsaved changes" );
expect( title ).toBeInTheDocument();
expect( title.tagName ).toBe( "H1" );
} );

it( "should have a description", () => {
const title = screen.queryByText( "There are unsaved changes in one or more steps of the first-time configuration. Leaving means that those changes will be lost. Are you sure you want to leave this page?" );
expect( title ).toBeInTheDocument();
const description = screen.queryByText( "There are unsaved changes in one or more steps of the first-time configuration. Leaving means that those changes will be lost. Are you sure you want to leave this page?" );
expect( description ).toBeInTheDocument();
expect( description.tagName ).toBe( "P" );
} );

it( "should call onClose when clicking the close button", () => {
fireEvent.click( screen.queryByText( "Close" ) );
expect( onClose ).toHaveBeenCalled();
} );

it( "should call onClose when clicking the continue editing button", () => {
fireEvent.click( screen.queryByText( "No, continue editing" ) );
expect( onClose ).toHaveBeenCalled();
} );

it( "should call onDiscard when clicking the leave page button", () => {
fireEvent.click( screen.queryByText( "Yes, leave page" ) );
expect( onDiscard ).toHaveBeenCalled();
} );

describe( "fallback props", () => {
it( "should not throw an error when the onClose and onDiscard props are not passed", () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit weird because it does not throw, it logs.

This tests the original issue. That the React warning is gone when not passing onClose or onDiscard.
Try to put the isRequired back on them to see if it then fails.

const currentImplementation = console.error;
console.error = jest.fn();

render( <UnsavedChangesModal
isOpen={ true }
title="Unsaved changes"
description="There are unsaved changes in one or more steps of the first-time configuration. Leaving means that those changes will be lost. Are you sure you want to leave this page?"
dismissLabel="No, continue editing"
discardLabel="Yes, leave page"
/> );

try {
expect( console.error ).not.toHaveBeenCalled();
} finally {
// Cleanup.
console.error = currentImplementation;
}
} );
} );
} );