Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly log out on Account Deletion #2692

Merged
merged 2 commits into from
Jan 13, 2021
Merged

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Jan 7, 2021

Resolves #2645

This also includes a fix for deleting GCS buckets which aren't empty.

how to test

@csweichel
Copy link
Contributor

csweichel commented Jan 8, 2021

/werft run

👍 started the job as gitpod-build-at-logout-on-delete.1

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Jan 12, 2021

/werft run

👍 started the job as gitpod-build-at-logout-on-delete.2

<DialogContent>
<Typography variant="body1">
<div>Your account has been deleted and your browser will be redirected to the logout.</div>
<div>Feel free to try out Gitpod again. No hard feelings 🙂</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

@gtsiolis @svenefftinge Is the messaging ok here?

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, 'reworded, because "to logout using the finish button below" is part of the problem solve in this PR.

Copy link
Contributor

@gtsiolis gtsiolis Jan 12, 2021

Choose a reason for hiding this comment

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

question: Do we need this extra (second) dialog for technical reasons?

  • If not, I'd suggest removing it and redirecting to www.gitpod.io after confirming deletion in the first dialog. In the future we could redirect to a dedicated the sign up page. Feel free to open a follow up issue for this.
  • If yes, here are some potential improvements that could go into this PR:
    • ..will be redirected to the logout does not sound accurate, right? We could skip mentioning the redirection.
    • We could rename the second sentence to something like Thanks for trying out Gitpod.

issue: This is out of the scope of this PR, but visible page contents during page reload are also an issue for anonymous users, see #2509.

Thanks for the ping @csweichel! 🏓

Copy link
Member Author

Choose a reason for hiding this comment

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

@gtsiolis, thanks!

great hint, and there isn't a technical reason anymore (as of this PR.)

follow up on this suggestion: would you expect to see any confirmation of the successful deletion at all, or is a plain redirection to www.gitpod.io just right?

Copy link
Member Author

Choose a reason for hiding this comment

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

@gtsiolis, please give it a try 🙏🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexTugarev looks good! I don't think we will need the second dialog in the long run but a loading indicator makes sense. Long term we could either use a loading button state (see screenshot below) or a page-wide progress bar like the one we use for environment (workspace) loading.

Screenshot 2021-01-12 at 3 48 38 PM (2)

Once we have a dedicated the sign up page we can consider redirecting users there so that both gitpod.io and self hosted users have a similar overall UX.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we will need the second dialog

which one?

maybe it's the structure of the React component which is a bit confusing? there are three ways to render a We are sorry to see you go dialog. I mean that's all the same dialog, but the content is changing to indicate progress and resolution.

Copy link
Contributor

@gtsiolis gtsiolis Jan 12, 2021

Choose a reason for hiding this comment

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

@AlexTugarev I'm referring to the second dialog shown after confirming deletion that contains the title We are sorry to see you go and the body message Redirecting....

What I meant earlier is that we do not need the second dialog at all. But leaving this as is for now is fine.

but the content is changing to indicate progress and resolution.

For this, see #2692 (comment). ❤️

Copy link
Member Author

@AlexTugarev AlexTugarev Jan 13, 2021

Choose a reason for hiding this comment

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

I see. I didn't even notice the last rendering of Redirecting..., maybe because the redirect happens to be fast enough. All three dialog states were a single dialog for me. The idea was to fill in the gap between dismissing the progress indicator and a potentially slow redirect of the browser.

But leaving this as is for now is fine.

Can you then give a thumb up on merging?

Signed-off-by: Alex Tugarev <alex.tugarev@typefox.io>
Signed-off-by: Alex Tugarev <alex.tugarev@typefox.io>
@AlexTugarev AlexTugarev merged commit aced836 into master Jan 13, 2021
@AlexTugarev AlexTugarev deleted the at/logout-on-delete branch January 13, 2021 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cant sign up after account deletion
4 participants