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

allow registration and login from guest to be cancellable #220

Merged
merged 3 commits into from
Mar 16, 2016

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Mar 15, 2016

@@ -1042,7 +1071,9 @@ module.exports = React.createClass({
registrationUrl={this.props.registrationUrl}
onLoggedIn={this.onRegistered}
onLoginClick={this.onLoginClick}
onRegisterClick={this.onRegisterClick} />
onRegisterClick={this.onRegisterClick}
onCancelClick={ MatrixClientPeg.get() && MatrixClientPeg.get().isGuest() ? this.onReturnToGuestClick : null }
Copy link
Member

Choose a reason for hiding this comment

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

How does this work? Will this not hide the back-to-app div if the user does anything to cause the matrix client to be replaced, like changing home servers?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, good catch. fixed.

@dbkr
Copy link
Member

dbkr commented Mar 16, 2016

Possibly a better way of doing this in the future might be to just make login / register use a separate matrixclient which only replaces the one in MatrixClientPeg on success, then we wouldn't have to worry about restoring the old login if the user cancelled (eg. presumably if the user clicks 'login', then refreshes the page, they will no longer be able to return to their guest session). This would probably be a more invasive change though, so appreciate we might not want to do this now.

@@ -275,11 +279,31 @@ module.exports = React.createClass({
screen: 'post_registration'
});
break;
case 'start_login_from_guest':
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be a separate thing? ie. could start_login just do if (MatrixClientPeg.get().isGuest()) {... and if something explicitly wants to log out first, it can just post the logout action first?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, it could be combined together. i guess it was a choice of separating the two flows with an isGuest() or adding a different dispatch target. keeping it separate gives us the option of logging in without preserving the guest details if desired (e.g. if you explicitly wanted to replace your current session with a new login). i'm not sure that one is a huge win over the other, but yell if you feel strongly.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, I don't really feel strongly.

@dbkr dbkr assigned ara4n and unassigned dbkr Mar 16, 2016
@ara4n ara4n assigned dbkr and unassigned ara4n Mar 16, 2016
@ara4n
Copy link
Member Author

ara4n commented Mar 16, 2016

@dbkr: PTAL

@dbkr
Copy link
Member

dbkr commented Mar 16, 2016

lgtm, modulo conflicts.

@dbkr dbkr assigned ara4n and unassigned dbkr Mar 16, 2016
ara4n added a commit that referenced this pull request Mar 16, 2016
allow registration and login from guest to be cancellable
@ara4n ara4n merged commit 74aad34 into develop Mar 16, 2016
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.

Need a way to get back to the existing guest session after clicking on 'Register' or 'login'
2 participants