-
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
Laushinka/team admin 8060 #8402
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8402 +/- ##
=========================================
+ Coverage 8.41% 11.17% +2.75%
=========================================
Files 33 18 -15
Lines 2340 993 -1347
=========================================
- Hits 197 111 -86
+ Misses 2138 880 -1258
+ Partials 5 2 -3
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
409e821
to
7e01108
Compare
Looking at this now, round two! 🥊 |
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.
Thanks for opening this PR, @laushinka! 🌟
UX looks good. Left some comments below that are complementary to what @jldec added in #8060 (comment).
add18ba
to
348b35d
Compare
b9d445a
to
6494c5c
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.
Thanks for updating this and addressing all comments, @laushinka!
Left some second review round comments below. 🥊
6494c5c
to
5c3e131
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.
Thanks, @laushinka! Round three! 🥊
</ItemField> | ||
<ItemField className="flex items-center my-auto"> | ||
<span className="text-gray-400 capitalize"> | ||
<DropDown contextMenuWidth="w-32" activeEntry={m.role} entries={[{ |
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: Did we decide to bring the role change action back?
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.
Yes, here as a nice to have for support issues[1]
67c888c
to
0cbb931
Compare
Running build again as the last one failed[1]. /werft run 👍 started the job as gitpod-build-laushinka-team-admin-8060.24 |
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 changes look great, @laushinka!
Last round! 🥊
One outstanding question is whether we'd like to include the linking back to the team from the project detail page in this PR or open a follow up issue for this. Your call. 🏓
Project in personal account | Project in team |
---|---|
@gtsiolis Thank you for remembering this! Will do it, since I'm still waiting for code reviews anyway 👏🏼 (Also, not sure what the "dismissed stale review is below" 😅) |
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.
Very nice work - thank you @laushinka
Description
Allows managing teams as admin.
Related Issue(s)
Fixes #8060
How to test
https://laushinka-team-admin-8060.staging.gitpod-dev.com/
/admin/teams
and search for teamsRelease Notes
Documentation