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

feat(project): collaboration tab has inside issues and merge requests #649

Merged
merged 1 commit into from
Nov 4, 2019

Conversation

vfried
Copy link
Contributor

@vfried vfried commented Oct 16, 2019

I created a new tab that has inside Issues that where previously Kus and also Merge Requests that where before Previous Changes.

I decided to refactor the way the issues look:
image

Before they looked like:
image

There is one layout issue i couldn't solve, when clicking on a merge request the "merge requests" tab gets unselected:
image

I can try and fix this when i'm back or leave it and create a bug for this.

I can't deploy to virginiatest because is being used but you can use this image to test: virfried/renku-ui:0.6.5-e392a4b

Closes #599
Closes #446

@vfried vfried requested a review from a team as a code owner October 16, 2019 16:12
@vfried vfried force-pushed the 599-collaboration-tab branch 6 times, most recently from ca5b041 to e392a4b Compare October 17, 2019 06:54
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.

I love the new collaboration tab, it's much cleaner than the previous solution. I added a couple of minor suggestions on the layout and I pushed a commit with a fix for the MergeRequestSuggestions component so that the buttons will not overflow. Fell free to change it or include it in your bigger commit (I didn't know how to suggest a change on untouched code lines).

I replicated the small issue you found when clicking on "Create Pending Change". I guess the problem is that the url changes to something like this /projects/lorenzo.cavazzi.tech/zurich-bikes-tutorial/collaboration/mergeRequests/undefined and no tab is underlined on the left because of the final /undefined in the url. EDIT: I guess the problem may be in "Project.js" at line 330 this.props.history.push(${this.getSubUrls().mergeRequestsOverviewUrl}/${newMRiid}) where newMRiid seems to be undefined.

I'll test a little more the Issues section but everything seems to work 😄

Screenshot_20191017_130142

src/issue/Issue.js Show resolved Hide resolved
src/project/Project.present.js Outdated Show resolved Hide resolved
@lorenzo-cavazzi
Copy link
Member

The Issues part is amazing! I didn't find any problem there.

I have only a request (that may be moved to a separate issue): when creating a new issue, I end up on a page that is not part of the new Collaboration tab. It would be great if the "New Issue" page would be moved there. The url could be changed from /issue_new to /collaboration/issues/new to be coherent with the current routing (/collaboration/issues*) and to simplify including it in the tab.

@lorenzo-cavazzi
Copy link
Member

this seems to address also #446

@vfried vfried force-pushed the 599-collaboration-tab branch 4 times, most recently from 2f5d919 to ecb4b75 Compare October 29, 2019 13:17
@vfried vfried self-assigned this Oct 29, 2019
@rokroskar
Copy link
Member

This is looking really good! Did you leave "pending change" in a few places on purpose? e.g.
image and image

@rokroskar
Copy link
Member

rokroskar commented Oct 29, 2019

One more minor thing - the x you use here is used in gitlab for failed CI jobs/pipelines - maybe we should use something else?
image

@vfried
Copy link
Contributor Author

vfried commented Oct 29, 2019

This is looking really good! Did you leave "pending change" in a few places on purpose? e.g.

Just to confuse the users haha 😏
I just changed the "pending changes" i missed.

Regarding the icons, that's a good point... I'll try to replace them for something better.

@vfried
Copy link
Contributor Author

vfried commented Oct 29, 2019

New deployment with latest changes (icons and replacement of some "pending changes" labels) https://virginiatest.dev.renku.ch/projects/virginiafriedrich/zurich-bikes-2/collaboration/issues/10/

@rokroskar
Copy link
Member

Hmm I'm wondering if it would be better to have the issues display in a single pane rather than side-by-side with the issue list? Visualizing an embedded notebook becomes very cluttered: https://virginiatest.dev.renku.ch/projects/virginiafriedrich/zurich-bikes-2/collaboration/issues/9/

On a wide screen it's fine but on a small-ish laptop it's probably not very usable.

@ciyer
Copy link
Contributor

ciyer commented Oct 30, 2019

This is not the end of the work on the issues or merge requests.

The main change in this issue is to get the "Pending Changes" and "Kus" renamed and placed together in a better location. Since the code was being touched anyway, there were some other improvements made as well.

I agree that issues and merge requests should take up the majority of the screen width, but I want to do that in another GitHub issue.

@ciyer ciyer force-pushed the 599-collaboration-tab branch 2 times, most recently from ab40af4 to 56c1dc0 Compare October 31, 2019 16:45
ciyer
ciyer previously approved these changes Oct 31, 2019
Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

Looks very nice! This cleans up the tabs quite a bit.

@vfried
Copy link
Contributor Author

vfried commented Nov 3, 2019

I added an icon that hides/shows the issues list allowing the user to have a better overview of the issue:

image

image

It would be nice to add a css transition to this but i am using display block/none and css doesn't suport transitons for the "display" property.

Testing available in virginiatest.dev.renku.ch

Addresses @rokroskar reqest on top.

@rokroskar
Copy link
Member

That looks great, thanks @vfried! Next, keyboard shortcuts for browsing through the issues :)

Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

Very nice improvements. This cleans up the project page and improves functionality, and will be a good base for building out the collaboration functionality.

@vfried vfried merged commit 5b5c44a into master Nov 4, 2019
@ciyer ciyer added this to the 0.7.1 milestone Nov 5, 2019
@ciyer ciyer deleted the 599-collaboration-tab branch November 5, 2019 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Collaboration Tab Buttons in Kus tab have a wrong format
4 participants