-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[teams] don't show error if already in team #5960
Conversation
/werft run 👍 started the job as gitpod-build-alex-redirect-users-to-the-members-5925.1 |
4247fed
to
df8e0a5
Compare
Looking at this now! 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UX works great, @AlexTugarev! ↩️
Left one comment regardingt the page we redirect below.
Approving to unblock merging but holding in case changing the redirect URL sounds better here.
/hold
const match = message && regExp.exec(message); | ||
if (match && match[1]) { | ||
const slug = match[1]; | ||
history.push(`/t/${slug}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: This redirects you to the team's workspaces page. Do you think it's more confusing to redirect to the members list instead as described in #5925?
However, redirecting to the team or members list sounds like a good MVC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunno. Let's redirect to the Members list instead. I'll ping you for a ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Please have another look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this now! 👀
LGTM label has been added. Git tree hash: 9ab15d60d0449d3d6886c3667045f192dfd2f95f
|
df8e0a5
to
e84990e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working like a charm, @AlexTugarev!
thought: I think it makes sense to go with the redirection to the members page as
Approving to ublocking merging but holding in case this needs someone to take a closer look at the code changes.
/hold
LGTM label has been added. Git tree hash: 0633ee7a5c48b1e9e055f80c18bfdabef4dada4e
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexTugarev, gtsiolis Associated issue: #5925 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Description
Directly redirect to the team if following an invitation link but already part of the team.
Related Issue(s)
Fixes #5925
How to test
Release Notes