Loading spinner no longer appear whan saving a resource

[tdipisa]

Description

When saving a resource (no matter if it is a map, dashboard or geostory) the loading spinner no longer appear. This is a regression. To check possibly what introduced it.

How to reproduce

Expected Result

The loading spinner should appear on bottom-left of the edit properties panel (also from home page).

Current Result

The loading spinner doesn't appear.

• Not browser related

Browser info

(use this site: https://www.whatsmybrowser.org/ for non expert users)

Browser Affected	Version
Internet Explorer
Edge
Chrome
Firefox
Safari

Other useful information

[dsuren1]

Investigation:
The visibility of the spinner is affected by this, where the border value is not formulated (it should be (@icon-size-md/10)) introduced as part of MS theme update issue.

However I don't see an implementation causing a regression that hides the loading spinner on click Save action. Checked it all the way to v2021, if I'm not wrong. Probably present before that.

Anyway, in this case, I believe the code needs to tweaked to address this specific scenario of showing spinner on save progress.
Maybe also take this time to also disable user interaction on the save modal fields when save is in progress?

@tdipisa Kindly let me know your thoughts. Thanks!

[tdipisa]

@dsuren1

Anyway, in this case, I believe the code needs to tweaked to address this specific scenario of showing spinner on save progress.

Yes,

Maybe also take this time to also disable user interaction on the save modal fields when save is in progress?

It would be better, yes.

Please provide an estimate in pts for both.

[tdipisa]

@dsuren1 3pts are too much now. Let's concentrate on fixing the regression by restoring the spinner. Then we can open an issue for the other point.

[offtherailz]

Hi @dsuren1 .
Probably you didn't noticed the implementation behind loading because it is hidden by redux/recompose and all the nested components.

Anyway it is present and I verified that fixing the style you linked like this:

.mapstore-circle-loader-with-css-variables(calc( @icon-size-md / 10), @icon-size-md, @theme-vars[loader-primary-color], @theme-vars[loader-primary-fade-color]);

[offtherailz]

Sorry,
I also did this change to make it visible and missed a save to delete this change,

diff --git a/web/client/components/resources/modals/Save.jsx b/web/client/components/resources/modals/Save.jsx
index 76ac8d227..986796a45 100644
--- a/web/client/components/resources/modals/Save.jsx
+++ b/web/client/components/resources/modals/Save.jsx
@@ -127,6 +127,13 @@ class SaveModal extends React.Component {
         dialogClassName: '',
         detailsComponent: require('./enhancers/handleDetails').default((DetailsComp))
     };
+
+    componentDidUpdate(prevProps) {
+        if (this.props.errors && this.props.errors.length > 0 && this.props.errors !== prevProps.errors && this.state?.loading) {
+            this.setState({loading: false});
+        }
+    }
+
     onCloseMapPropertiesModal = () => {
         this.props.onClose();
     }
@@ -144,7 +151,7 @@ class SaveModal extends React.Component {
 
         return (<Portal key="saveDialog">
             <ResizableModal
-                loading={this.props.loading}
+                loading={this.props.loading || this.state?.loading}
                 title={<Message msgId={this.props.title}/>}
                 show={this.props.show}
                 clickOutEnabled={this.props.clickOutEnabled}
@@ -157,8 +164,11 @@ class SaveModal extends React.Component {
                     disabled: this.props.resource.loading
                 }, {
                     text: <span><Message msgId={this.props.saveButtonLabel}/></span>,
-                    onClick: () => { this.onSave(); },
-                    disabled: !this.isValidForm() || this.props.loading || !this.props.canSave
+                    onClick: () => {
+                        this.setState({loading: true});
+                        this.onSave();
+                    },
+                    disabled: !this.isValidForm() || this.props.loading || this.state.loading || !this.props.canSave
                 }]}
                 showClose={!this.props.resource.loading}
                 onClose={this.onCloseMapPropertiesModal}>
diff --git a/web/client/themes/default/less/loaders.less b/web/client/themes/default/less/loaders.less
index 8f048681c..ce500a01b 100644
--- a/web/client/themes/default/less/loaders.less
+++ b/web/client/themes/default/less/loaders.less
@@ -56,7 +56,7 @@
     }
     .mapstore-inline-loader {
         margin: @padding-left-square-md;
-        .mapstore-circle-loader-with-css-variables(@icon-size-md/10, @icon-size-md, @theme-vars[loader-primary-color], @theme-vars[loader-primary-fade-color]);
+        .mapstore-circle-loader-with-css-variables(calc( @icon-size-md / 10), @icon-size-md, @theme-vars[loader-primary-color], @theme-vars[loader-primary-fade-color]);
     }
 }

This was showing the loader. Effectively the logic doesn't exist. This was a workaround that I forgot to delete while checking.
Anyway this workaround can not handle errors and so on.

So summarizing,
The spinner exists, but the logic to show it does not. Moreover the spiller was not working because of the css rule.
Very strange.
I'm afraid this was a mix of errors between migration to new save dialogs (old save dialog showed the spinner) and the theme update that created this confusion, that also confused me.

In fact the loading flag is passed, but only to show the details loading.

I should try to handle this in the Save modal as much as possible, so we can avoid to handle the loading state for every resource in a lot use cases, for each resource and save tool.

Even if not perfect, I provided here a sort of solution that fixes it in most of the cases.

diff --git a/web/client/components/resources/modals/Save.jsx b/web/client/components/resources/modals/Save.jsx
index 76ac8d227..a5cc4bb8a 100644
--- a/web/client/components/resources/modals/Save.jsx
+++ b/web/client/components/resources/modals/Save.jsx
@@ -127,6 +127,13 @@ class SaveModal extends React.Component {
         dialogClassName: '',
         detailsComponent: require('./enhancers/handleDetails').default((DetailsComp))
     };
+
+    componentDidUpdate(prevProps) {
+        if (this.props.errors && this.props.errors.length > 0 && this.props.errors !== prevProps.errors && this.state?.loading) {
+            this.setState({loading: false});
+        }
+    }
+
     onCloseMapPropertiesModal = () => {
         this.props.onClose();
     }
@@ -144,7 +151,7 @@ class SaveModal extends React.Component {
 
         return (<Portal key="saveDialog">
             <ResizableModal
-                loading={this.props.loading}
+                loading={this.props.loading || this.state?.loading}
                 title={<Message msgId={this.props.title}/>}
                 show={this.props.show}
                 clickOutEnabled={this.props.clickOutEnabled}
@@ -157,7 +164,10 @@ class SaveModal extends React.Component {
                     disabled: this.props.resource.loading
                 }, {
                     text: <span><Message msgId={this.props.saveButtonLabel}/></span>,
-                    onClick: () => { this.onSave(); },
+                    onClick: () => {
+                        this.setState({loading: true});
+                        this.onSave();
+                    },
                     disabled: !this.isValidForm() || this.props.loading || !this.props.canSave
                 }]}
                 showClose={!this.props.resource.loading}
diff --git a/web/client/themes/default/less/loaders.less b/web/client/themes/default/less/loaders.less
index 8f048681c..ce500a01b 100644
--- a/web/client/themes/default/less/loaders.less
+++ b/web/client/themes/default/less/loaders.less
@@ -56,7 +56,7 @@
     }
     .mapstore-inline-loader {
         margin: @padding-left-square-md;
-        .mapstore-circle-loader-with-css-variables(@icon-size-md/10, @icon-size-md, @theme-vars[loader-primary-color], @theme-vars[loader-primary-fade-color]);
+        .mapstore-circle-loader-with-css-variables(calc( @icon-size-md / 10), @icon-size-md, @theme-vars[loader-primary-color], @the
me-vars[loader-primary-fade-color]);
     }
 }

[dsuren1]

I should try to handle this in the Save modal as much as possible, so we can avoid to handle the loading state for every resource in a lot use cases, for each resource and save tool.

I agree @offtherailz