From ec63e18b42f3b5b195150c99060ae257ff84ed30 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 21 Mar 2017 18:40:41 +0000 Subject: [PATCH 1/4] Show spinner whilst processing recaptcha response The fact that we showed no feedback whilst submitting the captcha response was causing confusion on slower connections where this took a nontrivial amount of time. Takes a new flag from the js-sdk that indicates whether the request being made is a background request, presenting a spinner appropriately. Requires https://github.com/matrix-org/matrix-js-sdk/pull/396 --- src/components/structures/InteractiveAuth.js | 12 +++++++----- .../views/login/InteractiveAuthEntryComponents.js | 6 ++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/components/structures/InteractiveAuth.js b/src/components/structures/InteractiveAuth.js index 71fee883be..3dd34f51b4 100644 --- a/src/components/structures/InteractiveAuth.js +++ b/src/components/structures/InteractiveAuth.js @@ -140,9 +140,9 @@ export default React.createClass({ }); }, - _requestCallback: function(auth) { + _requestCallback: function(auth, background) { this.setState({ - busy: true, + busy: !background, errorText: null, stageErrorText: null, }); @@ -150,9 +150,11 @@ export default React.createClass({ if (this._unmounted) { return; } - this.setState({ - busy: false, - }); + if (background) { + this.setState({ + busy: false, + }); + } }); }, diff --git a/src/components/views/login/InteractiveAuthEntryComponents.js b/src/components/views/login/InteractiveAuthEntryComponents.js index 2d8abf9216..c4084facb2 100644 --- a/src/components/views/login/InteractiveAuthEntryComponents.js +++ b/src/components/views/login/InteractiveAuthEntryComponents.js @@ -160,6 +160,7 @@ export const RecaptchaAuthEntry = React.createClass({ submitAuthDict: React.PropTypes.func.isRequired, stageParams: React.PropTypes.object.isRequired, errorText: React.PropTypes.string, + busy: React.PropTypes.bool, }, _onCaptchaResponse: function(response) { @@ -170,6 +171,11 @@ export const RecaptchaAuthEntry = React.createClass({ }, render: function() { + if (this.props.busy) { + const Loader = sdk.getComponent("elements.Spinner"); + return ; + } + const CaptchaForm = sdk.getComponent("views.login.CaptchaForm"); var sitePublicKey = this.props.stageParams.public_key; return ( From e5a5ca9efcd8de1513fe927516c4e8eaaad69d2e Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 22 Mar 2017 10:53:15 +0000 Subject: [PATCH 2/4] Don't set busy state at all for background request --- src/components/structures/InteractiveAuth.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/components/structures/InteractiveAuth.js b/src/components/structures/InteractiveAuth.js index 3dd34f51b4..d520f4dff9 100644 --- a/src/components/structures/InteractiveAuth.js +++ b/src/components/structures/InteractiveAuth.js @@ -141,16 +141,20 @@ export default React.createClass({ }, _requestCallback: function(auth, background) { - this.setState({ - busy: !background, - errorText: null, - stageErrorText: null, - }); + // only set the busy flag if this is a non-background request + if (!background) { + this.setState({ + busy: true, + errorText: null, + stageErrorText: null, + }); + } return this.props.makeRequest(auth).finally(() => { if (this._unmounted) { return; } - if (background) { + // only unset the busy flag if this is a non-background request + if (!background) { this.setState({ busy: false, }); From 5ae7d5e4b218c633664ce093cdbdc803b0649b67 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 22 Mar 2017 11:13:00 +0000 Subject: [PATCH 3/4] More comments --- src/components/structures/InteractiveAuth.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/structures/InteractiveAuth.js b/src/components/structures/InteractiveAuth.js index d520f4dff9..fe7552d20f 100644 --- a/src/components/structures/InteractiveAuth.js +++ b/src/components/structures/InteractiveAuth.js @@ -141,7 +141,9 @@ export default React.createClass({ }, _requestCallback: function(auth, background) { - // only set the busy flag if this is a non-background request + // only set the busy flag if this is a non-background request, + // otherwise, the user initiated a request, so make the busy + // spinner appear and clear and existing error messages. if (!background) { this.setState({ busy: true, From 6a5682897446e362ce9d86867cb2f2010d5d965f Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 22 Mar 2017 11:25:33 +0000 Subject: [PATCH 4/4] Just return the promise if it's a bg request This makes the code a bit neater. --- src/components/structures/InteractiveAuth.js | 33 ++++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/components/structures/InteractiveAuth.js b/src/components/structures/InteractiveAuth.js index fe7552d20f..7c8a5b8065 100644 --- a/src/components/structures/InteractiveAuth.js +++ b/src/components/structures/InteractiveAuth.js @@ -141,26 +141,25 @@ export default React.createClass({ }, _requestCallback: function(auth, background) { - // only set the busy flag if this is a non-background request, - // otherwise, the user initiated a request, so make the busy - // spinner appear and clear and existing error messages. - if (!background) { - this.setState({ - busy: true, - errorText: null, - stageErrorText: null, - }); - } - return this.props.makeRequest(auth).finally(() => { + const makeRequestPromise = this.props.makeRequest(auth); + + // if it's a background request, just do it: we don't want + // it to affect the state of our UI. + if (background) return makeRequestPromise; + + // otherwise, manage the state of the spinner and error messages + this.setState({ + busy: true, + errorText: null, + stageErrorText: null, + }); + return makeRequestPromise.finally(() => { if (this._unmounted) { return; } - // only unset the busy flag if this is a non-background request - if (!background) { - this.setState({ - busy: false, - }); - } + this.setState({ + busy: false, + }); }); },