From 76dcd1a9208d1b72dea1efad00c3872136c13d30 Mon Sep 17 00:00:00 2001 From: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com> Date: Fri, 13 May 2022 12:23:42 -0700 Subject: [PATCH] Feat alert users with feedback instead of disabling next/save buttons (#68) * feat: implemented user feedback ErrorAlerts --- .../components/ErrorAlerts/ErrorAlert.jsx | 29 +++- .../ErrorAlerts/ErrorAlert.test.jsx | 26 ++- .../__snapshots__/ErrorAlert.test.jsx.snap | 17 +- .../FetchErrorAlert.test.jsx.snap | 2 + .../UploadErrorAlert.test.jsx.snap | 2 + .../ImageSettingsModal/AltTextControls.jsx | 26 ++- .../AltTextControls.test.jsx | 7 +- .../AltTextControls.test.jsx.snap | 58 ++++++- .../__snapshots__/index.test.jsx.snap | 34 +++- .../components/ImageSettingsModal/hooks.js | 106 ++++++++++-- .../ImageSettingsModal/hooks.test.js | 163 ++++++++++++++---- .../components/ImageSettingsModal/index.jsx | 23 ++- .../ImageSettingsModal/index.test.jsx | 5 +- .../components/ImageSettingsModal/messages.js | 12 ++ .../__snapshots__/index.test.jsx.snap | 11 ++ .../components/SelectImageModal/hooks.js | 39 +++-- .../components/SelectImageModal/hooks.test.js | 45 ++++- .../components/SelectImageModal/index.jsx | 20 ++- .../SelectImageModal/index.test.jsx | 11 +- .../components/SelectImageModal/messages.js | 41 +++-- 20 files changed, 541 insertions(+), 136 deletions(-) diff --git a/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.jsx b/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.jsx index 6a62d1b98..9b9dfc40d 100644 --- a/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.jsx +++ b/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.jsx @@ -11,7 +11,7 @@ export const hooks = { state: { isDismissed: (val) => React.useState(val), }, - dismissalHooks: ({ isError }) => { + dismissalHooks: ({ dismissError, isError }) => { const [isDismissed, setIsDismissed] = hooks.state.isDismissed(false); React.useEffect(() => { setIsDismissed(isDismissed && !isError); @@ -19,16 +19,21 @@ export const hooks = { [isError]); return { isDismissed, - dismissAlert: () => setIsDismissed(true), + dismissAlert: () => { + setIsDismissed(true); + dismissError(); + }, }; }, }; export const ErrorAlert = ({ + dismissError, + hideHeading, isError, children, }) => { - const { isDismissed, dismissAlert } = hooks.dismissalHooks({ isError }); + const { isDismissed, dismissAlert } = hooks.dismissalHooks({ dismissError, isError }); if (!isError || isDismissed) { return null; } @@ -39,17 +44,25 @@ export const ErrorAlert = ({ dismissible onClose={dismissAlert} > - - - + {!hideHeading + && ( + + + + )} {children} ); }; +ErrorAlert.defaultProps = { + dismissError: null, + hideHeading: false, +}; + ErrorAlert.propTypes = { + dismissError: PropTypes.func, + hideHeading: PropTypes.bool, isError: PropTypes.bool.isRequired, children: PropTypes.oneOfType([ PropTypes.arrayOf(PropTypes.node), diff --git a/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.test.jsx b/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.test.jsx index 436bf9e54..abeb6a5d3 100644 --- a/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.test.jsx +++ b/src/editors/containers/TextEditor/components/ErrorAlerts/ErrorAlert.test.jsx @@ -28,15 +28,20 @@ describe('ErrorAlert component', () => { beforeEach(() => { state.mock(); }); afterEach(() => { state.restore(); }); describe('dismissalHooks', () => { + const props = { + dismissError: jest.fn(), + isError: testValue, + }; beforeEach(() => { - hook = module.hooks.dismissalHooks({ isError: testValue }); + hook = module.hooks.dismissalHooks(props); }); it('returns isDismissed value, initialized to false', () => { expect(state.stateVals.isDismissed).toEqual(hook.isDismissed); }); - test('dismissAlert sets isDismissed to true', () => { + test('dismissAlert sets isDismissed to true and calls dismissError', () => { hook.dismissAlert(); expect(state.setState.isDismissed).toHaveBeenCalledWith(true); + expect(props.dismissError).toHaveBeenCalled(); }); test('On Render, calls setIsDismissed', () => { expect(React.useEffect.mock.calls.length).toEqual(1); @@ -51,20 +56,27 @@ describe('ErrorAlert component', () => { describe('Component', () => { describe('Snapshots', () => { let props; + const msg =

An Error Message

; beforeAll(() => { props = { - isError: false, + dismissError: jest.fn(), }; - jest.spyOn(module.hooks, 'dismissalHooks').mockImplementation((value) => ({ isError: value })); + jest.spyOn(module.hooks, 'dismissalHooks').mockImplementation(() => ({ + isDismissed: false, + dismissAlert: jest.fn().mockName('dismissAlert'), + })); }); afterAll(() => { jest.clearAllMocks(); }); - test('snapshot: is Null when no error (ErrorAlert)', () => { - expect(shallow(

An Error Message

)).toMatchSnapshot(); + test('snapshot: is Null when no error (ErrorAlert)', () => { + expect(shallow(

An Error Message

)).toMatchSnapshot(); }); test('snapshot: Loads children and component when error (ErrorAlert)', () => { - expect(shallow(

An Error Message

)).toMatchSnapshot(); + expect(shallow({msg})).toMatchSnapshot(); + }); + test('snapshot: Does not load heading when hideHeading is true', () => { + expect(shallow({msg})).toMatchSnapshot(); }); }); }); diff --git a/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/ErrorAlert.test.jsx.snap b/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/ErrorAlert.test.jsx.snap index c557aff11..8d8ecd143 100644 --- a/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/ErrorAlert.test.jsx.snap +++ b/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/ErrorAlert.test.jsx.snap @@ -1,10 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ErrorAlert component Component Snapshots snapshot: is Null when no error (ErrorAlert) 1`] = `""`; +exports[`ErrorAlert component Component Snapshots snapshot: Does not load heading when hideHeading is true 1`] = ` + +

+ An Error Message +

+
+`; exports[`ErrorAlert component Component Snapshots snapshot: Loads children and component when error (ErrorAlert) 1`] = ` @@ -14,10 +25,10 @@ exports[`ErrorAlert component Component Snapshots snapshot: Loads children and c id="authoring.texteditor.selectimagemodal.error.errorTitle" /> -

An Error Message

-
`; + +exports[`ErrorAlert component Component Snapshots snapshot: is Null when no error (ErrorAlert) 1`] = `""`; diff --git a/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/FetchErrorAlert.test.jsx.snap b/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/FetchErrorAlert.test.jsx.snap index c15744f8e..9cc5c939a 100644 --- a/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/FetchErrorAlert.test.jsx.snap +++ b/src/editors/containers/TextEditor/components/ErrorAlerts/__snapshots__/FetchErrorAlert.test.jsx.snap @@ -2,6 +2,8 @@ exports[`FetchErrorAlert Snapshots snapshot: is ErrorAlert with Message error (ErrorAlert) 1`] = ` + {validation.show + && ( + + + + )} @@ -46,10 +56,16 @@ export const AltTextControls = ({ ); AltTextControls.propTypes = { + error: PropTypes.shape({ + show: PropTypes.bool, + }).isRequired, isDecorative: PropTypes.bool.isRequired, - value: PropTypes.string.isRequired, setValue: PropTypes.func.isRequired, setIsDecorative: PropTypes.func.isRequired, + validation: PropTypes.shape({ + show: PropTypes.bool, + }).isRequired, + value: PropTypes.string.isRequired, // inject intl: intlShape.isRequired, }; diff --git a/src/editors/containers/TextEditor/components/ImageSettingsModal/AltTextControls.test.jsx b/src/editors/containers/TextEditor/components/ImageSettingsModal/AltTextControls.test.jsx index c45720330..55e3a1f12 100644 --- a/src/editors/containers/TextEditor/components/ImageSettingsModal/AltTextControls.test.jsx +++ b/src/editors/containers/TextEditor/components/ImageSettingsModal/AltTextControls.test.jsx @@ -19,9 +19,14 @@ describe('AltTextControls', () => { beforeEach(() => { props.setValue = jest.fn().mockName('props.setValue'); props.setIsDecorative = jest.fn().mockName('props.setIsDecorative'); + props.validation = { show: true }; }); describe('render', () => { - test('snapshot: isDecorative=true', () => { + test('snapshot: isDecorative=true errorProps.showSubmissionError=true', () => { + expect(shallow()).toMatchSnapshot(); + }); + test('snapshot: isDecorative=true errorProps.showSubmissionError=false', () => { + props.validation.show = false; expect(shallow()).toMatchSnapshot(); }); }); diff --git a/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/AltTextControls.test.jsx.snap b/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/AltTextControls.test.jsx.snap index 7a07ff7a4..15d516797 100644 --- a/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/AltTextControls.test.jsx.snap +++ b/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/AltTextControls.test.jsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`AltTextControls render snapshot: isDecorative=true 1`] = ` +exports[`AltTextControls render snapshot: isDecorative=true errorProps.showSubmissionError=false 1`] = ` @@ -17,6 +17,7 @@ exports[`AltTextControls render snapshot: isDecorative=true 1`] = ` className="mt-4.5" disabled={true} floatingLabel="Alt Text" + isInvalid={false} onChange={ Object { "hooks.onInputChange": [MockFunction props.setValue], @@ -44,3 +45,58 @@ exports[`AltTextControls render snapshot: isDecorative=true 1`] = ` `; + +exports[`AltTextControls render snapshot: isDecorative=true errorProps.showSubmissionError=true 1`] = ` + + + + + + + + + + + + + + +`; diff --git a/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/index.test.jsx.snap b/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/index.test.jsx.snap index ddf4b198b..754db4ee4 100644 --- a/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/TextEditor/components/ImageSettingsModal/__snapshots__/index.test.jsx.snap @@ -5,18 +5,17 @@ exports[`ImageSettingsModal render snapshot 1`] = ` close={[MockFunction props.close]} confirmAction={ )} > + + +