Sitelet https://web.archive.org/web/20190410172400im_/https://github.com/github/VisualStudio/pull/1787
Skip to content
Please note that GitHub no longer supports your web browser.

We recommend upgrading to the latest Google Chrome or Firefox.

Learn more

/VisualStudio

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

Refactored Clone Dialog #1787

Merged
merged 22 commits into from Sep 11, 2018

Conversation

Reviewers

@jcansdale jcansdale

Assignees

@donokuda donokuda

Labels
verified
Projects
None yet
Milestone
  2.5.6
4 participants
@grokys @donokuda @meaghanlewis @jcansdale
@grokys
Copy link
Contributor

grokys commented Jul 19, 2018 •
edited by meaghanlewis

Refactored the clone dialog, basing it initially on the clone dialog from desktop:

Features

  • Now uses GraphQL to load the list of repositories. This improves the load time of the list.
  • Separate GitHub and Enterprise tabs
  • A new "Clone from URL" tab

Screenshots

Particularly the URL clone view needs some input from @donokuda.

image

image

Scopes

GraphQL requires the read:org scope the access the user's repositories. This means that the user will have to log out and back in again to use the clone dialog. The following message will be shown in this case (needs input from @donokuda):

image

Caveats

  • You currently need to log out and back in because the GraphQL query needs the read:org scope which your previous login won't have. Need to implement a "You need to log out and back in again" flow in the dialog for this. This is now implemented
  • Only showing first 100 org repositories because of octokit/octokit.graphql.net#132 now fixed.

Depends on #1761

Closes #1757
@grokys
Initial reimplementation of Clone dialog. 
- Uses GraphQL
- Has a page for cloning from URL
Loading status checks…
0737c73

@donokuda donokuda self-assigned this Jul 19, 2018

@grokys
Return user's repositories and group in list. 
Previously were just returning repositories that belong to orgs. Now return the user's owned repositories as well, and group by owner in the list.

Currently returning only first 100 org repositories because of octokit/octokit.graphql.net#132 .
Loading status checks…
af3d0aa

@grokys grokys added the WIP label Jul 20, 2018

@jcansdale jcansdale referenced this pull request Jul 23, 2018

Closed

Using a slash in the clone dialog #1757

@meaghanlewis meaghanlewis added this to the 2.5.5 milestone Jul 30, 2018

@donokuda

This comment has been minimized.

Sign in to view
Copy link
Member

donokuda commented Jul 30, 2018

You currently need to log out and back in because the GraphQL query needs the read:org scope which your previous login won't have. Need to implement a "You need to log out and back in again" flow in the dialog for this.

In case it hasn't been considered yet, a potentially "quick & dirty" flow for this is to pop up a modal dialog on top of the clone dialog that give the individual the option to log out.

I'd imagine the flow to break down like this:

  1. I click on "Clone" from the GitHub section of Team Explorer's connect page
  2. A dialog appears explaining that I need to log out in order to proceed with two options: [Cancel] [Log out]
  3. I click "Log out," the extension attempts to log me out.
  • On success ✅: the login modal appears and I step through that flow. (Ideally: After logging in, I'm presented with the clone dialog since that's the action I've been meaning to do in the first place.)
  • On failure ❌: Another dialog appears with the error and two options: [Cancel] [Try again]
    • Selecting "Try again" attempts logging out again.

@meaghanlewis meaghanlewis modified the milestones: 2.5.5, 2.5.6 Aug 13, 2018

grokys added some commits Aug 27, 2018

@grokys
Merge branch 'refactor/sdk-csproj' into refactor/clone 
 Conflicts:
	src/GitHub.App/Resources.resx
	src/GitHub.VisualStudio.UI/Properties/AssemblyInfo.cs
Loading status checks…
f026189
@grokys
Use RepositoryAffiliation.Owner.
19fcd1e
@grokys
Address TODOs. 
Now that we've updated Octokit.GraphQL.
Loading status checks…
8252cb8
@grokys
Separate scopes into minimum and requested.
4858036
@grokys
Display useful message for AggregateException.
Loading status checks…
655aca5
@grokys
Merge branch 'refactor/sdk-csproj' into refactor/clone
Loading status checks…
5008995
@grokys
Fix analyser warnings in LoginManager.
322df9b
@grokys
Return LoginResult from LoginManager. 
So we can return the scopes as well as the user.
a8a1bb5
@grokys
Store scopes in the Connection.
a9f01eb
@grokys
Implemented log out/back in flow.
Loading status checks…
7429224
@grokys
Merge branch 'refactor/sdk-csproj' into refactor/clone
Loading status checks…
ad4627b
@grokys
Fix failing test.
Loading status checks…
ddafcd3

@sguthals sguthals referenced this pull request Sep 5, 2018

Closed

Development Sprint: October 1st Release #1899

meaghanlewis and others added some commits Sep 5, 2018

@meaghanlewis
Merge branch 'master' into refactor/clone
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
b5d814b
Andreia Gaita
Merge branch 'master' into refactor/clone
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
0193b45
@meaghanlewis

This comment has been minimized.

Sign in to view
Copy link
Contributor

meaghanlewis commented Sep 6, 2018

This looks great to me @grokys !

One thing that I ran into was when I wanted to enter the username and repo to clone - ex github/quality. I wasn't signed into my GitHub account at the time - only GitHub Enterprise. This login dialog appeared:

screen shot 2018-09-06 at 2 29 11 pm

I authenticated with it, but it didn't actually sign me in and I got an error like this Error encountered while cloning the remote repository: Git failed with a fatal error. unable to access 'https://github.com/github/quality/': The requested URL returned error: 403.

Is it possible to show our Sign in dialog here instead to avoid this from possibly confusing users?
@grokys

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

grokys commented Sep 10, 2018

@meaghanlewis hmm yes, I'm not quite sure how to deal with that. If the repository is publicly visible then this clone should actually work. We could make a check to see if we have a connection to the server that the clone is being requested from, but again, the clone may actually work.

I notice desktop does the same thing in this situation and shows a login dialog. I'm tempted to say that this is expected behavior.
@jcansdale

jcansdale approved these changes Sep 10, 2018
View changes

Copy link
Collaborator

jcansdale left a comment •
edited

Sorry, wrong PR. 😊
@jcansdale
Merge branch 'master' into refactor/clone
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
e5ef59e
@jcansdale
Merge branch 'master' into refactor/clone
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
2224fb3
@jcansdale

jcansdale reviewed Sep 10, 2018
View changes

Copy link
Collaborator

jcansdale left a comment

Changing Approve to Comment.

@jcansdale jcansdale referenced this pull request Sep 10, 2018

Merged

Include owner in default path when cloning a repository #1906

@meaghanlewis

This comment has been minimized.

Sign in to view
Copy link
Contributor

meaghanlewis commented Sep 10, 2018

hey @grokys im okay to leave the behavior the same when trying to clone a private repo while signed out of a GitHub account. This might just be an edge case and there is a workaround anyway to sign into the account first and then clone.
@grokys
Implement browse button.
Loading status checks…
f5369ba
@meaghanlewis

This comment has been minimized.

Sign in to view
Copy link
Contributor

meaghanlewis commented Sep 10, 2018

One more question - could you add a message for when someone tries to clone a repo they already have?

Currently, the clone button is disabled when trying to clone an existing repo but it might not be clear why.
screen shot 2018-09-10 at 8 01 53 am

In the old (current) clone dialog we at least showed an icon in the path to indicate it can't be cloned to that location.
screen shot 2018-09-10 at 8 10 06 am

And in desktop, they have an explicit message saying the destination already exists. Could we also show a message similar to this?
screen shot 2018-09-10 at 8 01 39 am
@grokys
Include InfoPanel template. 
To make error messages appear in clone dialog.
Loading status checks…
9784ce6

@meaghanlewis meaghanlewis added the verified label Sep 10, 2018

@grokys grokys changed the title WIP: Refactored Clone Dialog Refactored Clone Dialog Sep 11, 2018

@grokys grokys merged commit 0e64838 into master Sep 11, 2018
2 checks passed

2 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@meaghanlewis meaghanlewis removed the WIP label Sep 26, 2018

grokys added a commit that referenced this pull request Jan 10, 2019

@grokys
Fix displaying errors in GitHub pane. 
#1787 refactored `InfoPanel` but didn't update `GitHubPaneView` with its new behavior. So:

- Don't initialy set `infoPanel.Visibility` to `Collapsed`: `InfoPanel` will handle this itself
- Set `ShowCloseButton` to show the `x` button to close the error

Fixes #2172
Fixes #2058
Loading status checks…
c53fa14

@grokys grokys referenced this pull request Jan 10, 2019

Merged

Fix displaying errors in GitHub pane. #2173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.