From 2766c9a28c4baed63dce05833f077492bab0575d Mon Sep 17 00:00:00 2001
From: Ben Wheeler <wheeler.benjamin@gmail.com>
Date: Wed, 14 Nov 2018 23:35:21 -0500
Subject: [PATCH] saving, creation errors use alerts; alerts css improvements

---
 src/components/alerts/alert.css               | 15 +++--
 src/components/alerts/alert.jsx               | 25 +++----
 src/components/close-button/close-button.css  | 14 ++--
 src/lib/alerts/index.jsx                      | 22 +++++++
 src/lib/project-saver-hoc.jsx                 | 17 +++--
 src/reducers/alerts.js                        |  3 +
 src/reducers/project-state.js                 | 35 ++++++++--
 .../reducers/project-state-reducer.test.js    | 66 +++++++++++++++++--
 8 files changed, 156 insertions(+), 41 deletions(-)

diff --git a/src/components/alerts/alert.css b/src/components/alerts/alert.css
index 5cefca72b..ee4c71058 100644
--- a/src/components/alerts/alert.css
+++ b/src/components/alerts/alert.css
@@ -8,8 +8,8 @@
     flex-direction: row;
     overflow: hidden;
     align-items: left;
-    border-radius: 8px;
-    padding: 8px;
+    border-radius: $space;
+    padding: $space;
     margin-bottom: 7px;
 }
 
@@ -26,16 +26,18 @@
 }
 
 .alert-icon {
-    margin-right: 5px;
+    margin: $space;
     vertical-align: middle;
 }
 
 .alert-message {
     color: #555;
     font-weight: bold;
-    font-size: 12px;
-    line-height: 22pt;
+    font-size: .75rem;
+    line-height: 14px;
     width: 100%;
+    display: flex;
+    align-items: center;
 }
 
 .alert-close-button {
@@ -43,11 +45,10 @@
 }
 
 .alert-close-button-container {
-    margin-top: 7px;
-    margin-right: 4px;
     outline-style:none;
     width: 30px;
     height: 30px;
+    align-self: center;
 }
 
 .connection-button {
diff --git a/src/components/alerts/alert.jsx b/src/components/alerts/alert.jsx
index 391dd297e..89d7f6b90 100644
--- a/src/components/alerts/alert.jsx
+++ b/src/components/alerts/alert.jsx
@@ -30,18 +30,19 @@ const AlertComponent = ({
     >
         <div className={styles.alertMessage}>
             {/* TODO: implement Rtl handling */}
-            {iconSpinner && (
-                <Spinner />
-            )}
-            {iconURL && (
-                <img
-                    className={styles.alertIcon}
-                    src={iconURL}
-                />
-            )}
-            {message}
-            &nbsp;
-            {content}
+            <div className={styles.alertIcon}>
+                {iconSpinner && (
+                    <Spinner />
+                )}
+                {iconURL && (
+                    <img src={iconURL} />
+                )}
+            </div>
+            <div>
+                {message}
+                &nbsp;
+                {content}
+            </div>
         </div>
         {showReconnect ? (
             <button
diff --git a/src/components/close-button/close-button.css b/src/components/close-button/close-button.css
index 90bd473ce..836067c7e 100644
--- a/src/components/close-button/close-button.css
+++ b/src/components/close-button/close-button.css
@@ -7,7 +7,7 @@
     justify-content: center;
 
     overflow: hidden;  /* Mask the icon animation */
-    background-color: $ui-black-transparent;
+    /* background-color: $ui-black-transparent; */
     border-radius: 50%;
     font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
     user-select: none;
@@ -17,30 +17,30 @@
 
 .close-button.large:hover {
     transform: scale(1.1, 1.1);
-    box-shadow: 0 0 0 4px $ui-black-transparent;
+    /* box-shadow: 0 0 0 4px $ui-black-transparent; */
 }
 
 .close-button.large.orange:hover {
     transform: scale(1.1, 1.1);
-    box-shadow: 0px 0px 0px 4px hsla(29, 100%, 54%, 0.2);
+    /* box-shadow: 0px 0px 0px 4px hsla(29, 100%, 54%, 0.2); */
 }
 
 .small {
     width: 0.825rem;
     height: 0.825rem;
     color: $ui-white;
-    background-color: $motion-primary;
+    /* background-color: $motion-primary; */
 }
 
 .large {
     width: 1.75rem;
     height: 1.75rem;
-    box-shadow: 0 0 0 2px $ui-black-transparent;
+    /* box-shadow: 0 0 0 2px $ui-black-transparent; */
 }
 
 .large.orange {
-    background-color: hsla(29, 100%, 54%, 0.2);
-    box-shadow: 0px 0px 0px 2px hsla(29, 100%, 54%, 0.2);
+    /* background-color: hsla(29, 100%, 54%, 0.2);
+    box-shadow: 0px 0px 0px 2px hsla(29, 100%, 54%, 0.2); */
 }
 
 .close-icon {
diff --git a/src/lib/alerts/index.jsx b/src/lib/alerts/index.jsx
index a8f8b0b81..bb4d60206 100644
--- a/src/lib/alerts/index.jsx
+++ b/src/lib/alerts/index.jsx
@@ -34,6 +34,28 @@ const alerts = [
         iconSpinner: true,
         level: AlertLevels.SUCCESS
     },
+    {
+        alertId: 'creatingError',
+        content: (
+            <FormattedMessage
+                defaultMessage="Could not create the project. Please try again!"
+                description="Message indicating that project could not be created"
+                id="gui.alerts.creatingError"
+            />
+        ),
+        level: AlertLevels.WARN
+    },
+    {
+        alertId: 'savingError',
+        content: (
+            <FormattedMessage
+                defaultMessage="Could not save the project. Please try again!"
+                description="Message indicating that project could not be saved"
+                id="gui.alerts.savingError"
+            />
+        ),
+        level: AlertLevels.WARN
+    },
     {
         alertId: 'saveSuccess',
         clearList: ['saving'],
diff --git a/src/lib/project-saver-hoc.jsx b/src/lib/project-saver-hoc.jsx
index 4d44e143a..f63eb8b0d 100644
--- a/src/lib/project-saver-hoc.jsx
+++ b/src/lib/project-saver-hoc.jsx
@@ -85,7 +85,10 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 })
                 .catch(err => {
                     // NOTE: should throw up a notice for user
-                    this.props.onProjectError(`Saving the project failed with error: ${err}`);
+                    if (this.props.isManualUpdating) {
+                        this.props.onShowAlert('savingError');
+                    }
+                    this.props.onProjectError(err);
                 });
         }
         createNewProjectToStorage () {
@@ -96,7 +99,8 @@ const ProjectSaverHOC = function (WrappedComponent) {
                     this.props.onCreatedProject(response.id.toString(), this.props.loadingState);
                 })
                 .catch(err => {
-                    this.props.onProjectError(`Creating a new project failed with error: ${err}`);
+                    this.props.onShowAlert('savingError');
+                    this.props.onProjectError(err);
                 });
         }
         createCopyToStorage () {
@@ -111,7 +115,8 @@ const ProjectSaverHOC = function (WrappedComponent) {
                     this.props.onCreatedProject(response.id.toString(), this.props.loadingState);
                 })
                 .catch(err => {
-                    this.props.onProjectError(`Creating a project copy failed with error: ${err}`);
+                    this.props.onShowAlert('savingError');
+                    this.props.onProjectError(err);
                 });
         }
         createRemixToStorage () {
@@ -126,7 +131,8 @@ const ProjectSaverHOC = function (WrappedComponent) {
                     this.props.onCreatedProject(response.id.toString(), this.props.loadingState);
                 })
                 .catch(err => {
-                    this.props.onProjectError(`Remixing a project failed with error: ${err}`);
+                    this.props.onShowAlert('savingError');
+                    this.props.onProjectError(err);
                 });
         }
         /**
@@ -173,6 +179,7 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 onCreateProject,
                 onManualUpdateProject,
                 onProjectError,
+                onShowAlert,
                 onShowCreateSuccessAlert,
                 onShowCreatingAlert,
                 onShowSaveSuccessAlert,
@@ -208,6 +215,7 @@ const ProjectSaverHOC = function (WrappedComponent) {
         onCreatedProject: PropTypes.func,
         onManualUpdateProject: PropTypes.func,
         onProjectError: PropTypes.func,
+        onShowAlert: PropTypes.func,
         onShowCreateSuccessAlert: PropTypes.func,
         onShowCreatingAlert: PropTypes.func,
         onShowSaveSuccessAlert: PropTypes.func,
@@ -239,6 +247,7 @@ const ProjectSaverHOC = function (WrappedComponent) {
         onCreateProject: () => dispatch(createProject()),
         onManualUpdateProject: () => dispatch(manualUpdateProject()),
         onProjectError: error => dispatch(projectError(error)),
+        onShowAlert: alertType => dispatch(showStandardAlert(alertType)),
         onShowCreateSuccessAlert: () => dispatch(showStandardAlert('createSuccess')),
         onShowCreatingAlert: () => dispatch(showStandardAlert('creating')),
         onShowSaveSuccessAlert: () => dispatch(showStandardAlert('saveSuccess')),
diff --git a/src/reducers/alerts.js b/src/reducers/alerts.js
index cb74f2b29..d542c4037 100644
--- a/src/reducers/alerts.js
+++ b/src/reducers/alerts.js
@@ -33,6 +33,9 @@ const reducer = function (state, action) {
                 newList = newList.filter(curAlert => (
                     !alertData.clearList || alertData.clearList.indexOf(curAlert.alertId)
                 ));
+                if (action.data && action.data.message) {
+                    newAlert.message = action.data.message;
+                }
 
                 newAlert.content = alertData.content;
                 newAlert.iconURL = alertData.iconURL;
diff --git a/src/reducers/project-state.js b/src/reducers/project-state.js
index c063138e4..86ae65836 100644
--- a/src/reducers/project-state.js
+++ b/src/reducers/project-state.js
@@ -188,7 +188,7 @@ const reducer = function (state, action) {
             return state;
         }
         // if setting the default project id, specifically fetch that project
-        if (action.projectId === defaultProjectId) {
+        if (action.projectId === defaultProjectId || action.projectId === null) {
             return Object.assign({}, state, {
                 loadingState: LoadingState.FETCHING_NEW_DEFAULT,
                 projectId: defaultProjectId
@@ -278,21 +278,42 @@ const reducer = function (state, action) {
         }
         return state;
     case START_ERROR:
-    // NOTE: we should introduce handling in components for showing ERROR state
+        // fatal errors: there's no correct editor state for us to show
         if ([
-            LoadingState.AUTO_UPDATING,
-            LoadingState.CREATING_COPY,
-            LoadingState.CREATING_NEW,
             LoadingState.FETCHING_NEW_DEFAULT,
             LoadingState.FETCHING_WITH_ID,
             LoadingState.LOADING_VM_NEW_DEFAULT,
-            LoadingState.LOADING_VM_WITH_ID,
+            LoadingState.LOADING_VM_WITH_ID
+        ].includes(state.loadingState)) {
+            return Object.assign({}, state, {
+                loadingState: LoadingState.ERROR,
+                error: action.error
+            });
+        }
+        // non-fatal errors: there's no correct editor state for us to show
+        if ([
+            LoadingState.AUTO_UPDATING,
+            LoadingState.CREATING_COPY,
             LoadingState.MANUAL_UPDATING,
             LoadingState.REMIXING,
             LoadingState.UPDATING_BEFORE_NEW
         ].includes(state.loadingState)) {
             return Object.assign({}, state, {
-                loadingState: LoadingState.ERROR,
+                loadingState: LoadingState.SHOWING_WITH_ID,
+                error: action.error
+            });
+        }
+        // non-fatal error; state to show depends on whether project we're showing
+        // has an id or not
+        if (state.loadingState === LoadingState.CREATING_NEW) {
+            if (state.projectId === defaultProjectId || state.projectId === null) {
+                return Object.assign({}, state, {
+                    loadingState: LoadingState.SHOWING_WITHOUT_ID,
+                    error: action.error
+                });
+            }
+            return Object.assign({}, state, {
+                loadingState: LoadingState.SHOWING_WITH_ID,
                 error: action.error
             });
         }
diff --git a/test/unit/reducers/project-state-reducer.test.js b/test/unit/reducers/project-state-reducer.test.js
index 94659c5d6..d5375a3f9 100644
--- a/test/unit/reducers/project-state-reducer.test.js
+++ b/test/unit/reducers/project-state-reducer.test.js
@@ -287,20 +287,78 @@ test('projectError from various states should show error', () => {
             error: null,
             loadingState: startState
         };
-        const action = projectError({message: 'Error string'});
+        const action = projectError('Error string');
+        const resultState = projectStateReducer(initialState, action);
+        expect(resultState.error).toEqual('Error string');
+    }
+});
+
+test('fatal projectError should show error state', () => {
+    const startStates = [
+        LoadingState.FETCHING_NEW_DEFAULT,
+        LoadingState.FETCHING_WITH_ID,
+        LoadingState.LOADING_VM_NEW_DEFAULT,
+        LoadingState.LOADING_VM_WITH_ID
+    ];
+    for (const startState of startStates) {
+        const initialState = {
+            error: null,
+            loadingState: startState
+        };
+        const action = projectError('Error string');
         const resultState = projectStateReducer(initialState, action);
         expect(resultState.loadingState).toBe(LoadingState.ERROR);
-        expect(resultState.error).toEqual({message: 'Error string'});
     }
 });
 
+test('non-fatal projectError should show normal state', () => {
+    const startStates = [
+        LoadingState.AUTO_UPDATING,
+        LoadingState.CREATING_COPY,
+        LoadingState.MANUAL_UPDATING,
+        LoadingState.REMIXING,
+        LoadingState.UPDATING_BEFORE_NEW
+    ];
+    for (const startState of startStates) {
+        const initialState = {
+            error: null,
+            loadingState: startState
+        };
+        const action = projectError('Error string');
+        const resultState = projectStateReducer(initialState, action);
+        expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID);
+    }
+});
+
+test('projectError when creating new while viewing project with id should show project with id', () => {
+    const initialState = {
+        error: null,
+        loadingState: LoadingState.CREATING_NEW,
+        projectId: '12345'
+    };
+    const action = projectError('Error string');
+    const resultState = projectStateReducer(initialState, action);
+    expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID);
+});
+
+test('projectError when creating new while logged out, looking at default project should show default project', () => {
+    const initialState = {
+        error: null,
+        loadingState: LoadingState.CREATING_NEW,
+        projectId: '0'
+    };
+    const action = projectError('Error string');
+    const resultState = projectStateReducer(initialState, action);
+    expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID);
+});
+
 test('projectError from showing project should show error', () => {
     const initialState = {
         error: null,
         loadingState: LoadingState.FETCHING_WITH_ID
     };
-    const action = projectError({message: 'Error string'});
+    const action = projectError('Error string');
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.ERROR);
-    expect(resultState.error).toEqual({message: 'Error string'});
+    expect(resultState.error).toEqual('Error string');
 });
-- 
GitLab