Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Show spinner whilst processing recaptcha response #767

Merged
merged 4 commits into from
Mar 22, 2017

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Mar 21, 2017

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 matrix-org/matrix-js-sdk#396

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 matrix-org/matrix-js-sdk#396
this.setState({
busy: true,
busy: !background,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I'm not quite grokking this. why do we only set busy if not background?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this was a bit silly (and buggy). I've updated it so it doesn't set the state at all for background requests, and commented it.

@richvdh richvdh assigned dbkr and unassigned richvdh Mar 21, 2017
@dbkr dbkr assigned richvdh and unassigned dbkr Mar 22, 2017
errorText: null,
stageErrorText: null,
});
// only set the busy flag if this is a non-background request
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, I'm still not grokking it. A comment that says "do X" doesn't help me understand why X is the right thing to do :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any better?

@richvdh richvdh assigned dbkr and unassigned richvdh Mar 22, 2017
@dbkr dbkr assigned richvdh and unassigned dbkr Mar 22, 2017
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. up to you if you want to rearrange it a bit as discussed irl

@richvdh richvdh assigned dbkr and unassigned richvdh Mar 22, 2017
This makes the code a bit neater.
@dbkr dbkr merged commit fc9928c into develop Mar 22, 2017
martindale pushed a commit to FabricLabs/matrix-react-sdk that referenced this pull request Dec 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants