Skip to content

[teams] make Gitpod icon team-aware #5854

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 27, 2021
Merged

[teams] make Gitpod icon team-aware #5854

merged 2 commits into from
Sep 27, 2021

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Sep 24, 2021 •
edited
Loading

Description

Related Issue(s)

Fixes #5519

How to test

Release Notes

NONE

@AlexTugarev AlexTugarev requested review from gtsiolis and jankeromnes and removed request for JanKoehnlein September 24, 2021 09:13
@AlexTugarev AlexTugarev mentioned this pull request Sep 24, 2021
Closed
@AlexTugarev
Copy link
Member Author

AlexTugarev commented Sep 24, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-alex-persist-team-selection-on-5519.2

@AlexTugarev AlexTugarev marked this pull request as ready for review September 24, 2021 13:38
gtsiolis
gtsiolis approved these changes Sep 24, 2021
View reviewed changes
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.

Thanks @AlexTugarev! Navigation changes work like a charm! 🔮

Couldn't reproduce defaulting to personal account when importing from GitLab.com. 😭

We're still missing persisting team selection when reloading / as described in #5519 but we could improve that later or in a follow-up issue. ➿

Approving to unblock merging but holding in case we need someone to take a closer look at the code changes.

/hold

components/server/ee/src/gitlab/gitlab-app-support.ts Outdated
@@ -22,6 +22,13 @@ export class GitLabAppSupport {
const api = new Gitlab({ oauthToken });

const result: ProviderRepository[] = [];
const ownersRepos: ProviderRepository[] = [];

const identity = params.user.identities.find(i => i.authProviderId === "Public-GitLab");
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: This is related #5331, right? I'm still seeing a different top-level group selected by default when trying to add a new project from GitLab.com. Expected? 😭

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh no, that commit shouldn't go to that branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

@roboquat roboquat added the lgtm label Sep 24, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 59a8935dacf04a1d362aabde5202b373bbf29120

@AlexTugarev AlexTugarev force-pushed the alex/persist-team-selection-on-5519 branch from c02bd10 to c267ce3 Compare September 24, 2021 14:02
@roboquat roboquat removed the lgtm label Sep 24, 2021
@roboquat
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@AlexTugarev AlexTugarev marked this pull request as draft September 24, 2021 14:21
@AlexTugarev
Copy link
Member Author

AlexTugarev commented Sep 27, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-alex-persist-team-selection-on-5519.4

@gtsiolis
Copy link
Contributor

gtsiolis commented Sep 27, 2021 •
edited by werft-gitpod-dev-com bot
Loading

@AlexTugarev let me know if you need another review round for this. 🏓

@AlexTugarev
Copy link
Member Author

@gtsiolis, yes please!

@AlexTugarev AlexTugarev marked this pull request as ready for review September 27, 2021 11:50
gtsiolis
gtsiolis reviewed Sep 27, 2021
View reviewed changes
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.

Flawless! Thanks @AlexTugarev! ⭕

5oeif3

@AlexTugarev
Copy link
Member Author

/approve

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlexTugarev, gtsiolis

Associated issue: #5519

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@AlexTugarev
Copy link
Member Author

/hold cancel

@roboquat roboquat merged commit d28d433 into main Sep 27, 2021
@roboquat roboquat deleted the alex/persist-team-selection-on-5519 branch September 27, 2021 12:43
jankeromnes
jankeromnes reviewed Sep 28, 2021
View reviewed changes
components/dashboard/src/Menu.tsx
if (team) {
// updating last team selection
try {
localStorage.setItem('team-selection', team.slug);
Copy link
Contributor

Choose a reason for hiding this comment

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

And how can you revert back to User is selected by default? Current code reads like if you select a team once, you'll forever have a team selected by default, never your own user.

Also, I find restoring this selection on page load quite dangerous, and would personally prefer we rely on URLs for correct team selection (i.e. no local storage + restore on root).

@AlexTugarev
Copy link
Member Author

And how can you revert back to User is selected by default? Current code reads like if you select a team once, you'll forever have a team selected by default, never your own user.

fair point. let me check, but I think unsetting when navigating away from a team's page sounds reasonable.
cc. @gtsiolis

Also, I find restoring this selection on page load quite dangerous, and would personally prefer we rely on URLs for correct team selection (i.e. no local storage + restore on root).

that's the whole point. if you recently visited a team's page and then open gitpod.io/ in a new tab, you continue with the team page.

@AlexTugarev AlexTugarev mentioned this pull request Sep 28, 2021
Merged
@gtsiolis gtsiolis added component: dashboard feature: teams and projects [DEPRECATED] Please, use feature: organizations or feature: projects labels instead. team: webapp Issue belongs to the WebApp team labels Sep 29, 2021
@gtsiolis gtsiolis mentioned this pull request Oct 4, 2021
Closed
@gtsiolis gtsiolis mentioned this pull request Jan 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtsiolis gtsiolis gtsiolis approved these changes

@jankeromnes jankeromnes jankeromnes left review comments

Assignees

@gtsiolis gtsiolis

Labels
approved component: dashboard feature: teams and projects [DEPRECATED] Please, use feature: organizations or feature: projects labels instead. release-note-none size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Persist team selection on reload
4 participants
@AlexTugarev @roboquat @gtsiolis @jankeromnes