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

[dashboard] Improve team members page #4571

Merged
merged 3 commits into from
Jun 25, 2021
Merged

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Jun 22, 2021

Follow-up to #4490

  • Fix memberSince date
  • Replace paths /new-team and /join-team with /teams/new and /teams/join respectively
  • Hide most of the top menu when creating a new team
  • Implement removing team members & leaving teams
  • Implement searching & filtering team members
  • Implement changing team member roles

@jankeromnes jankeromnes force-pushed the jx/improve-team-members branch 3 times, most recently from cc92bde to 4c98804 Compare June 22, 2021 10:00
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jun 22, 2021

Seems like Werft fell asleep 😴:

Screenshot 2021-06-22 at 12 03 29

/werft run

👍 started the job as gitpod-build-jx-improve-team-members.4

@gtsiolis
Copy link
Contributor

Reposting here two more functionalities that didn't make it in #4421 in case these could fit into the scope of this PR:

  1. Searching Members
  2. Filtering Members

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jun 22, 2021

Reposting here two more functionalities that didn't make it in #4421

Good catch, thanks!

  1. Searching Members
  2. Filtering Members

Personally, I'd be more comfortable leaving out extra complexity, and waiting for user feature requests before implementing these.

I'm a bit wary of premature optimization. To me, a super-simple system is always much better than a slightly-more-complicated system, and I generally prefer leaving out as many bells and whistles from our product as possible until it becomes obvious that a particular bell or whistle is an absolute must-have. (E.g. I would have even left out the expiring team invite system, until a user requested that such a thing be implemented, but maybe I'm a bit too dogmatic about this.)

However, I'd understand if you consider that a members list without search/filter isn't up to our quality standards. So, I'm happy to have them in-scope for this PR. 😇 💯

@jankeromnes jankeromnes force-pushed the jx/improve-team-members branch 2 times, most recently from 639bca0 to 47301c8 Compare June 22, 2021 14:36
@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 22, 2021

... I'd be more comfortable leaving out extra complexity ...

@jankeromnes all valid points! Keeping these two functionalities out of the scope of this PR until further demand, sounds good! Posting a friendly reminder to remove these non-functional elements! 🎗️

@jankeromnes jankeromnes force-pushed the jx/improve-team-members branch 2 times, most recently from 94cc1aa to e03f524 Compare June 22, 2021 15:37
@jankeromnes jankeromnes marked this pull request as ready for review June 22, 2021 15:38
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jun 23, 2021

Looks like a random Werft network flake:

[components/gitpod-db:dbtest-init] docker: Error response from daemon: driver failed programming external connectivity on endpoint reverent_fermi (7d5a87ed18a8762b7bb1854564f8c90a0019ed07de98a1289148f42fb6db87f2): Bind for 0.0.0.0:23306 failed: port is already allocated.
[testdb] 2021-06-23T09:59:15.477626Z 9 [Note] Aborted connection 9 to db: 'gitpod' user: 'gitpod' host: '127.0.0.1' (Got an error reading communication packets)
[testdb] 2021-06-23T09:59:15.477676Z 6 [Note] Aborted connection 6 to db: 'gitpod' user: 'gitpod' host: '127.0.0.1' (Got an error reading communication packets)
[testdb] 2021-06-23T09:59:15.477641Z 8 [Note] Aborted connection 8 to db: 'gitpod' user: 'gitpod' host: '127.0.0.1' (Got an error reading communication packets)
[testdb] 2021-06-23T09:59:15.477699Z 5 [Note] Aborted connection 5 to db: 'gitpod' user: 'gitpod' host: '127.0.0.1' (Got an error reading communication packets)
[testdb] 2021-06-23T09:59:15.477645Z 7 [Note] Aborted connection 7 to db: 'gitpod' user: 'gitpod' host: '127.0.0.1' (Got an error reading communication packets)

/werft run

👍 started the job as gitpod-build-jx-improve-team-members.10

@jankeromnes jankeromnes force-pushed the jx/improve-team-members branch 2 times, most recently from c618b83 to a961299 Compare June 23, 2021 13:46
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jun 23, 2021

Ok, this is rebased and works as intended! 🚀

Please take a look @AlexTugarev / @gtsiolis / @svenefftinge 👀

Copy link
Member

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

Works nicely! 🎉

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 25, 2021

Looking at this now! 👀

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 25, 2021

/werft run

👍 started the job as gitpod-build-jx-improve-team-members.14

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Exciting new stuff! Thanks for adding this and also implementing the search and membership filter despite #4571 (comment), @jankeromnes! Hopefully, these (search and filter) were worth adding. 🔮

Left some comments that we could split into follow-up issues.

One last question: Somewhere we split the team dropdown into two distinct elements. Could we restore this as one component? See relevant discussion (internal). ❓

components/dashboard/src/teams/Members.tsx Outdated Show resolved Hide resolved
components/dashboard/src/teams/Members.tsx Outdated Show resolved Hide resolved
components/dashboard/src/teams/Members.tsx Outdated Show resolved Hide resolved
components/dashboard/src/teams/Members.tsx Outdated Show resolved Hide resolved
</div>
<div className="flex-1" />
<div className="py-3 pl-3">
<DropDown prefix="Role: " contextMenuWidth="w-32" activeEntry={'All'} entries={[{
<DropDown prefix="Role: " contextMenuWidth="w-32" activeEntry={roleFilter === 'owner' ? 'Owner' : (roleFilter === 'member' ? 'Member' : 'All')} entries={[{
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Works like a charm! 🔮

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'm quite proud of the clean and minimal solution I found. 😁

@@ -78,19 +110,19 @@ export default function() {
<div className="py-4">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 16 16" width="16" height="16"><path fill="#A8A29E" d="M6 2a4 4 0 100 8 4 4 0 000-8zM0 6a6 6 0 1110.89 3.477l4.817 4.816a1 1 0 01-1.414 1.414l-4.816-4.816A6 6 0 010 6z"/></svg>
</div>
<input type="search" placeholder="Search Members" onChange={() => { /* TODO */ }} />
<input type="search" placeholder="Search Members" onChange={e => setSearchText(e.target.value)} />
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Flawless! 🌟

components/dashboard/src/Menu.tsx Show resolved Hide resolved
components/dashboard/src/Menu.tsx Show resolved Hide resolved
@jankeromnes jankeromnes force-pushed the jx/improve-team-members branch 4 times, most recently from f6e0b47 to e9fb3ba Compare June 25, 2021 15:11
@codecov
Copy link

codecov bot commented Jun 25, 2021

Codecov Report

Merging #4571 (e9fb3ba) into main (5651cfc) will increase coverage by 35.74%.
The diff coverage is n/a.

❗ Current head e9fb3ba differs from pull request most recent head c476bef. Consider uploading reports for the commit c476bef to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##           main    #4571       +/-   ##
=========================================
+ Coverage      0   35.74%   +35.74%     
=========================================
  Files         0       14       +14     
  Lines         0     3858     +3858     
=========================================
+ Hits          0     1379     +1379     
- Misses        0     2361     +2361     
- Partials      0      118      +118     
Flag Coverage Δ
components-ws-manager-app 35.74% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/ws-manager/pkg/manager/monitor.go 0.00% <0.00%> (ø)
components/ws-manager/pkg/manager/headless.go 44.00% <0.00%> (ø)
components/ws-manager/pkg/manager/probe.go 0.00% <0.00%> (ø)
components/ws-manager/pkg/manager/manager.go 22.84% <0.00%> (ø)
components/ws-manager/pkg/manager/create.go 78.53% <0.00%> (ø)
components/ws-manager/pkg/manager/status.go 72.14% <0.00%> (ø)
...s/ws-manager/pkg/manager/internal/grpcpool/pool.go 74.46% <0.00%> (ø)
components/ws-manager/pkg/manager/imagespec.go 0.00% <0.00%> (ø)
...-manager/pkg/manager/internal/workpool/workpool.go 100.00% <0.00%> (ø)
components/ws-manager/pkg/manager/manager_ee.go 0.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5651cfc...c476bef. Read the comment docs.

- Fix memberSince date
- Replace paths /{new,join}-team with /teams/{new,join}
- Implement minimal top menu layout for full-page forms (e.g. new team/project)
- Implement removing members & leaving teams
- Implement member search & role filter
- Implement changing team member roles
Follow-up to #4589
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants