From 0c377377fa5b9110ea1e2cedb98cba333313980e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Aug 2019 00:52:12 +0200 Subject: [PATCH 1/7] Use "prop()" instead of "attr()" to get properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- js/views/callinfoview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 90ca480b329..30b4bf57cb8 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -243,7 +243,7 @@ * Share link */ toggleLinkCheckbox: function() { - var isPublic = this.ui.linkCheckbox.attr('checked') === 'checked'; + var isPublic = this.ui.linkCheckbox.prop('checked'); this.model.setPublic(isPublic); }, From f56122ae787f433f94eaea60a5a8277c6b0089be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Aug 2019 01:02:40 +0200 Subject: [PATCH 2/7] Wait for the server response to set the room as public or private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now the model was updated as soon as the checkbox was modified, which triggered a change event that in turn caused the UI to be updated. However, if the room could not be successfully set as public or private nothing was done and the model as well as the UI were kept with the wrong state (at least, until the model is fetched again). To prevent this now the model waits for a successful server response to update the attributes; in case of failure only the checkbox needs to be restored, as it is the only UI element that changed due to the direct interaction of the user (in case of success the whole view will be rendered again due to the room type change, so no need to do anything explicitly in that case). As the model is not changed until the successful server response is received now the checkbox is replaced by a working icon while waiting for the answer. In a similar way the checkbox is also disabled to prevent further requests while the previous one is ongoing. Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 12 ++++++++++++ js/views/callinfoview.js | 19 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/css/style.scss b/css/style.scss index 70a16bd136e..c816e1cf3f5 100644 --- a/css/style.scss +++ b/css/style.scss @@ -913,6 +913,18 @@ input[type="password"] { .link-checkbox-label { white-space: nowrap; padding: 12px; + + /* If ".icon-loading-small" is set hide the checkbox and show a + * loading icon instead */ + &.icon-loading-small:before { + background-image: none !important; + background-color: transparent !important; + border-color: transparent !important; + } + &.icon-loading-small:after { + top: 21px; + left: 23px; + } } .button { cursor: pointer; diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 30b4bf57cb8..a3687246539 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -65,6 +65,7 @@ 'shareLinkOptions': '.share-link-options', 'clipboardButton': '.clipboard-button', 'linkCheckbox': '.link-checkbox', + 'linkCheckboxLabel': '.link-checkbox-label', 'callButton': 'div.call-button', @@ -245,7 +246,23 @@ toggleLinkCheckbox: function() { var isPublic = this.ui.linkCheckbox.prop('checked'); - this.model.setPublic(isPublic); + this.ui.linkCheckbox.prop('disabled', true); + this.ui.linkCheckboxLabel.addClass('icon-loading-small'); + + this.model.setPublic(isPublic, { + wait: true, + error: function() { + this.ui.linkCheckbox.prop('checked', !isPublic); + this.ui.linkCheckbox.prop('disabled', false); + this.ui.linkCheckboxLabel.removeClass('icon-loading-small'); + + if (isPublic) { + OC.Notification.show(t('spreed', 'Error occurred while making the room public'), {type: 'error'}); + } else { + OC.Notification.show(t('spreed', 'Error occurred while making the room private'), {type: 'error'}); + } + }.bind(this) + }); }, /** From 5d6ed51f42c03ab5a5b78a8fadc51c455de1c710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Aug 2019 10:29:44 +0200 Subject: [PATCH 3/7] Provide feedback while waiting for the server to set the password MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to its special nature the password is not stored in an attribute of the model. Nevertheless, a "changed:password" (always with an empty value) was triggered as soon as the password was submited. However, as there is no password attribute that event is not taken into account anywhere and the UI was not updated until the server response was received (with further updates once the rooms are fetched again). To provide feedback to the user that the password is being set now the confirm button is replaced by a working icon while waiting for the answer. In a similar way the input is also disabled to prevent further requests while the previous one is ongoing. Note that due to the aforementioned behaviour it is not really necessary to wait for the server response to update the model with the new password (as it will not be updated anyway), but it is done for consistency with other requests. Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 34 +++++++++++++++++++++- js/views/callinfoview.js | 17 ++++++++++- js/views/templates.js | 2 +- js/views/templates/callinfoview.handlebars | 1 + 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/css/style.scss b/css/style.scss index c816e1cf3f5..47f21007fb6 100644 --- a/css/style.scss +++ b/css/style.scss @@ -83,6 +83,9 @@ * It is assumed that the icon will have the standard width for buttons in * inputs of 34px. However, further adjustments may be needed for the input and * the padding depending on the context where they are used. + * + * The confirm icon can have a sibling loading icon to switch to (by hiding one + * icon and showing the other) while the operation is in progress. */ input[type="text"], input[type="password"] { @@ -114,6 +117,32 @@ input[type="password"] { &:active:not(:disabled) { opacity: 1; } + + + .icon-loading-small { + /* Mimic size set in server for confirm button. */ + width: 34px; + height: 34px; + padding: 7px 6px; + margin-top: 3px; + margin-bottom: 3px; + + position: absolute; + top: 0; + right: 3px; + } + } +} + +.menuitem input[type="text"], +.menuitem input[type="password"] { + & + .icon-confirm + .icon-loading-small { + /* Mimic size set in server for inputs in menu items. */ + min-width: 44px; + max-height: 40px; + margin: 2px 0; + + /* Override padding set in server for icons in menu items. */ + padding: 7px 6px; } } @@ -953,11 +982,14 @@ input[type="password"] { .password-form { position: relative; - .password-confirm { + .password-confirm, + .password-loading { /* Inputs in menu items do not have a right margin, so * it does not need to be compensated. */ right: 0; + } + .password-confirm { /* Needed to override an important rule set in the * server. */ background-color: transparent !important; diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index a3687246539..eaea5f6c070 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -75,6 +75,7 @@ 'passwordOption': '.password-option', 'passwordInput': '.password-input', 'passwordConfirm': '.password-confirm', + 'passwordLoading': '.password-loading', 'menu': '.password-menu', }, @@ -273,14 +274,28 @@ var newPassword = this.ui.passwordInput.val().trim(); + this.ui.passwordInput.prop('disabled', true); + this.ui.passwordConfirm.addClass('hidden'); + this.ui.passwordLoading.removeClass('hidden'); + + var restoreState = function() { + this.ui.passwordInput.prop('disabled', false); + this.ui.passwordConfirm.removeClass('hidden'); + this.ui.passwordLoading.addClass('hidden'); + }.bind(this); + this.model.setPassword(newPassword, { + wait: true, success: function() { this.ui.passwordInput.val(''); + restoreState(); OC.hideMenus(); }.bind(this), error: function() { + restoreState(); + OC.Notification.show(t('spreed', 'Error occurred while setting password'), {type: 'error'}); - } + }.bind(this) }); }, diff --git a/js/views/templates.js b/js/views/templates.js index f88adf8c804..88c9cc532bf 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -73,7 +73,7 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.hasPassword : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.program(10, data, 0),"data":data})) != null ? stack1 : "") + " password-option\">\n
\n \n \n
\n \n \n \n \n \n"; + + "\">\n \n \n \n \n \n \n \n \n"; },"8":function(container,depth0,helpers,partials,data) { return "icon-password"; },"10":function(container,depth0,helpers,partials,data) { diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index b39c4cd07b7..8d1600eba3a 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -26,6 +26,7 @@ + From bb1ad01cec4da74aa5838b92b73926ca861178b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Aug 2019 10:56:23 +0200 Subject: [PATCH 4/7] Do not mark the password field as required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An empty password can be sent to remove the current password of the room, which does not match with the "required" semantics. Moreover, the CSS styles show a red border in invalid input fields, so the red border was shown too when removing the password despite being a valid operation. Signed-off-by: Daniel Calviño Sánchez --- js/views/templates.js | 2 +- js/views/templates/callinfoview.handlebars | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/views/templates.js b/js/views/templates.js index 88c9cc532bf..383e1493275 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -71,7 +71,7 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.hasPassword : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.program(10, data, 0),"data":data})) != null ? stack1 : "") + "\">\n
\n
    \n
  • \n \n
    \n \n \n \n \n \n \n
    \n
    • \n
\n
\n \n"; },"8":function(container,depth0,helpers,partials,data) { diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index 8d1600eba3a..0af592581fc 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -23,7 +23,7 @@
  • -