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.

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

Add links to GitLab #890

Merged
merged 3 commits into from
Apr 3, 2020
Merged

Add links to GitLab #890

merged 3 commits into from
Apr 3, 2020

Conversation

ciyer
Copy link
Contributor

@ciyer ciyer commented Apr 1, 2020 •
edited
Loading

Add links to the following GitLab URLs:

  • User Profile
  • User Settings
  • GitLab Home

image

Can be tested at https://sekhar.dev.renku.ch/

Fix #867

@ciyer ciyer requested a review from a team as a code owner April 1, 2020 13:50
@ciyer ciyer changed the title Add links to GitLab WIP: Add links to GitLab Apr 1, 2020
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
refactor: consolidate implementation of ExternalLink and ExternalDocs…
40fdce9 
…Link (#890)

re #867
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
feat(user): add links to gitlab (#890)
8ce51f4 
Add links to:

- User Profile
- User Settings
- GitLab Home

Fix #867
@ciyer ciyer force-pushed the 867-gitlab-link branch from 609b817 to 8ce51f4 Compare April 1, 2020 13:53
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
refactor: consolidate implementation of ExternalIconLink and External…
f66149c 
…DocsIconLink (#890)

Re #867
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
feat(user): add links to gitlab (#890)
eec075f 
Add links to:

- User Profile
- User Settings
- GitLab Home

Fix #867
@ciyer ciyer force-pushed the 867-gitlab-link branch from 8ce51f4 to eec075f Compare April 1, 2020 14:22
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
refactor: consolidate implementation of ExternalLink and ExternalDocs…
7db8030 
…Link (#890)

re #867
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
refactor: consolidate implementation of ExternalIconLink and External…
a8e62b2 
…DocsIconLink (#890)

Re #867
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
feat(user): add links to gitlab (#890)
7d57cfb 
Add links to:

- User Profile
- User Settings
- GitLab Home

Fix #867
@ciyer ciyer force-pushed the 867-gitlab-link branch from eec075f to 7d57cfb Compare April 1, 2020 14:32
@ciyer ciyer changed the title WIP: Add links to GitLab Add links to GitLab Apr 1, 2020
@emmjab emmjab requested review from emmjab and removed request for emmjab April 1, 2020 16:14
@emmjab
Copy link
Contributor

emmjab commented Apr 1, 2020

Thanks @ciyer !! I think this is good for now!

Slightly worried that the GitLab icon will be hard to describe for new-to-gitlab users, but we can include the icon in the docs I guess. It does do a good job saving space, and the first link once you click it does say GitLab.

Just noticed from testing that the help menu doesn't have a link to the docs~ but that's in another PR, will comment there.

@vfried
Copy link
Contributor

vfried commented Apr 1, 2020 •
edited
Loading

I think that having the link to profile and settings under the user and under the gitlab sign could be a bit redundant, especially since both links are right. next to each other...

image
image

Shall we remove some of this links somewhere?

vfried
vfried reviewed Apr 1, 2020
View reviewed changes
Copy link
Contributor

@vfried vfried left a comment

Choose a reason for hiding this comment

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

I left a comment on top and a small code correction here, besides that it looks good.

@vfried
Copy link
Contributor

vfried commented Apr 1, 2020 •
edited
Loading

Thanks @ciyer !! I think this is good for now!

Slightly worried that the GitLab icon will be hard to describe for new-to-gitlab users, but we can include the icon in the docs I guess. It does do a good job saving space, and the first link once you click it does say GitLab.

Just noticed from testing that the help menu doesn't have a link to the docs~ but that's in another PR, will comment there.

I am all in for helping users have an easy beginning with renku, but i think we ned to draw a line about how much we "hide" gitlab inside renku, since for other users having gitlab is also seen as a benefit. I think that at some point someone using renku that has no idea about gitlab will have to make a quick search to see what it it.

(Don't want to sound like an antisocial person that sits behind the computer writing hate comments in newspapers haha I just want for us to think about this thing, you can for sure disagree)

ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
refactor: consolidate implementation of ExternalIconLink and External…
6c1da15 
…DocsIconLink (#890)

Re #867
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
feat(user): add links to gitlab (#890)
acab664 
Add links to:

- User Profile
- User Settings
- GitLab Home

Fix #867
@ciyer ciyer force-pushed the 867-gitlab-link branch from 7d57cfb to acab664 Compare April 1, 2020 17:48
@ciyer ciyer added this to the sprint-2020-03-27 milestone Apr 1, 2020
ciyer pushed a commit that referenced this pull request Apr 1, 2020
@cramakri
feat(user): add links to gitlab (#890)
5aadf5c 
Add links to:

- User Profile
- User Settings
- GitLab Home

Fix #867
@ciyer ciyer force-pushed the 867-gitlab-link branch from acab664 to 5aadf5c Compare April 1, 2020 17:56
@emmjab
Copy link
Contributor

emmjab commented Apr 1, 2020

I am all in for helping users have an easy beginning with renku, but i think we ned to draw a line about how much we "hide" gitlab inside renku, since for other users having gitlab is also seen as a benefit. I think that at some point someone using renku that has no idea about gitlab will have to make a quick search to see what it it.

wait i think we're on the same side? 😅
basically what i mean is that it's hard for someone to google for an icon -- my suggestion would be to replace the fox icon with the word "GitLab"

I think that having the link to profile and settings under the user and under the gitlab sign could be a bit redundant, especially since both links are right. next to each other...

Also confused --> before the "Settings" and "Profile" were renku settings and profile in the "User" menu.

@cramakri
feat(user): add links to gitlab (#890)
795af1d 
Add links to:

- User Profile
- User Settings
- GitLab Home

Fix #867
@ciyer ciyer force-pushed the 867-gitlab-link branch from 5aadf5c to 795af1d Compare April 2, 2020 10:05
@ciyer
Copy link
Contributor Author

ciyer commented Apr 2, 2020 •
edited
Loading

@vfried

I think that having the link to profile and settings under the user and under the gitlab sign could be a bit redundant, especially since both links are right. next to each other...

image
image

Shall we remove some of this links somewhere?

You are right. In this case, since they are so close to each-other, it's confusing to have the links redundantly. I have removed them from the user menu and just left them in the GitLab menu.

@ciyer
Copy link
Contributor Author

ciyer commented Apr 2, 2020

Also confused --> before the "Settings" and "Profile" were renku settings and profile in the "User" menu.

There is no difference ATM between Renku Settings and GitLab settings, so those links lead to the same place.

@vfried
Copy link
Contributor

vfried commented Apr 2, 2020

Cool, i think it's better if we only have them in one place.

lorenzo-cavazzi
lorenzo-cavazzi approved these changes Apr 3, 2020
View reviewed changes
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Looks good 👍

src/landing/NavBar.js
@@ -49,6 +50,20 @@ class RenkuNavBar extends Component {
}
}

function gitLabSettingsUrlFromProfileUrl(webUrl) {
// Yes, the settings URL ends with 'profile'; the profile URL ends with the username
Copy link
Member

Choose a reason for hiding this comment

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

🤦‍♂️

@ciyer ciyer merged commit 4538959 into master Apr 3, 2020
ciyer pushed a commit that referenced this pull request Apr 3, 2020
@cramakri @ciyer
refactor: consolidate implementation of ExternalLink and ExternalDocs…
8b22d8c 
…Link (#890)

re #867
ciyer pushed a commit that referenced this pull request Apr 3, 2020
@cramakri @ciyer
refactor: consolidate implementation of ExternalIconLink and External…
9ecdabd 
…DocsIconLink (#890)

Re #867
@ciyer ciyer deleted the 867-gitlab-link branch April 3, 2020 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vfried vfried vfried left review comments

@lorenzo-cavazzi lorenzo-cavazzi lorenzo-cavazzi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
sprint-2020-03-27
Development

Successfully merging this pull request may close these issues.

link to gitlab from renkulab nav bar
5 participants
@ciyer @emmjab @vfried @lorenzo-cavazzi @cramakri