fix: keep the visualization name when re-saving after edit (#1273)

When clicking Save on a stored visualization, keep the same name and
avoid the default Untitled name be used instead.

Since no change is actually lost when navigating in and out of an
interpretation's view, remove the confirm dialog.

Also, when answering No to the confirm dialog (meaning i don't want to
navigate to the new URL) restore the previous URL with history.goBack()
so that the app really is in the same state as before the confirm
dialog.

packages/app/src/actions/index.js

@@ -201,7 +201,11 @@ export const tDoSaveVisualization = ({ name, description }, copy) => async (
         }
 
         visualization.name =
+            // name provided in the Save dialog
             name ||
+            // existing name when saving the same modified visualization
+            visualization.name ||
+            // new visualization with no name provided in Save dialog
             i18n.t('Untitled {{visualizationType}} visualization, {{date}}', {
                 visualizationType: getDisplayNameByVisType(visualization.type),
                 date: new Date().toLocaleDateString(undefined, {

packages/app/src/components/App.js

@@ -72,16 +72,22 @@ export class App extends Component {
         return false
     }
 
+    parseLocation = location => {
+        const pathParts = location.pathname.slice(1).split('/')
+        const id = pathParts[0]
+        const interpretationId = pathParts[2]
+        return { id, interpretationId }
+    }
+
     loadVisualization = async location => {
         const { store } = this.context
 
         if (location.pathname.length > 1) {
             // /currentAnalyticalObject
             // /${id}/
             // /${id}/interpretation/${interpretationId}
-            const pathParts = location.pathname.slice(1).split('/')
-            const id = pathParts[0]
-            const interpretationId = pathParts[2]
+            const { id, interpretationId } = this.parseLocation(location)
+
             const urlContainsCurrentAOKey = id === CURRENT_AO_KEY
 
             if (urlContainsCurrentAOKey) {
@@ -153,18 +159,33 @@ export class App extends Component {
 
         this.unlisten = history.listen(location => {
             const isSaving = location.state?.isSaving
+            const isOpening = location.state?.isOpening
+            const { interpretationId } = this.parseLocation(location)
+
             if (
+                // currently editing
                 getVisualizationState(
                     this.props.visualization,
                     this.props.current
                 ) === STATE_DIRTY &&
-                this.state.locationToConfirm !== location &&
+                // wanting to navigate elsewhere
+                this.state.previousLocation !== location.pathname &&
+                // currently *not* viewing an interpretation
+                !(this.props.interpretation.id || interpretationId) &&
+                // not saving
                 !isSaving
             ) {
                 this.setState({ locationToConfirm: location })
             } else {
+                if (
+                    isSaving ||
+                    isOpening ||
+                    this.state.previousLocation !== location.pathname
+                ) {
+                    this.loadVisualization(location)
+                }
+
                 this.setState({ locationToConfirm: null })
-                this.loadVisualization(location)
             }
         })
 
@@ -258,21 +279,27 @@ export class App extends Component {
                             <ButtonStrip end>
                                 <Button
                                     secondary
-                                    onClick={() =>
+                                    onClick={() => {
                                         this.setState({
                                             locationToConfirm: null,
                                         })
-                                    }
+
+                                        history.goBack()
+                                    }}
                                 >
                                     {i18n.t('No, cancel')}
                                 </Button>
 
                                 <Button
-                                    onClick={() =>
-                                        history.push(
+                                    onClick={() => {
+                                        this.loadVisualization(
                                             this.state.locationToConfirm
                                         )
-                                    }
+
+                                        this.setState({
+                                            locationToConfirm: null,
+                                        })
+                                    }}
                                     primary
                                 >
                                     {i18n.t('Yes, leave')}
@@ -291,7 +318,7 @@ export class App extends Component {
 const mapStateToProps = state => ({
     settings: fromReducers.fromSettings.sGetSettings(state),
     current: fromReducers.fromCurrent.sGetCurrent(state),
-    interpretations: fromReducers.fromVisualization.sGetInterpretations(state),
+    interpretation: fromReducers.fromUi.sGetUiInterpretation(state),
     ui: fromReducers.fromUi.sGetUi(state),
     visualization: sGetVisualization(state),
     snackbar: fromReducers.fromSnackbar.sGetSnackbar(state),
@@ -326,6 +353,7 @@ App.propTypes = {
     clearVisualization: PropTypes.func,
     current: PropTypes.object,
     d2: PropTypes.object,
+    interpretation: PropTypes.object,
     location: PropTypes.object,
     ouLevels: PropTypes.array,
     setCurrentFromUi: PropTypes.func,

packages/app/src/components/MenuBar/MenuBar.js

@@ -19,7 +19,7 @@ import styles from './styles/MenuBar.module.css'
 const onOpen = id => {
     const path = `/${id}`
     if (history.location.pathname === path) {
-        history.replace(path)
+        history.replace({ pathname: path, state: { isOpening: true } })
     } else {
         history.push(path)
     }

packages/app/src/components/__tests__/App.spec.js

@@ -112,13 +112,32 @@ describe('App', () => {
             })
         })
 
-        it('reloads visualization when same pathname pushed', done => {
+        it('reloads visualization when opening the same visualization', done => {
             props.location.pathname = '/fluttershy'
 
             app()
 
             setTimeout(() => {
-                history.replace('/fluttershy')
+                history.replace({
+                    pathname: '/fluttershy',
+                    state: { isOpening: true },
+                })
+                expect(actions.tDoLoadVisualization).toBeCalledTimes(2)
+
+                done()
+            })
+        })
+
+        it('reloads visualization when same pathname pushed when saving', done => {
+            props.location.pathname = '/fluttershy'
+
+            app()
+
+            setTimeout(() => {
+                history.replace({
+                    pathname: '/fluttershy',
+                    state: { isSaving: true },
+                })
                 expect(actions.tDoLoadVisualization).toBeCalledTimes(2)
 
                 done()