Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@shana
Copy link
Contributor

@shana shana commented Apr 18, 2016

When the user is logged out, we should probably not try to show all the data, and instead inform them that they should log in. This PR (initially) contains a stub of a logged out view, to flesh out. To see it, log out of GitHub and make sure you have a GitHub repo selected.

@shana shana force-pushed the feature/pr/logged-out-view branch from 5f99ea1 to 3b85d2f Compare April 20, 2016 08:46
@shana
Copy link
Contributor Author

shana commented Apr 20, 2016

@donokuda I rebased this branch against feature/pr/views and enabled the logged out view. This is what I get now:

screen shot 2016-04-20 at 10 38 30 am

We might want to dock it to the top so it doesn't appear floating in the middle of the pane, perhaps?

Since this is our UI and not Team Explorer, how about calling it "Login to GitHub" instead of "Connect to GitHub"? "Connect" is a Team Explorer thing which is specific to what it does in TFS (connecting to remote TFS projects).

Also, do we want to have more information on why the user should login? Would that be worth it?

@pmn @haacked Thoughts?

@shana
Copy link
Contributor Author

shana commented Apr 20, 2016

On a side note, it is possible to have the login happen directly in the pane and not as a separate dialog, if we want to do that at some point. Just fyi.

@shana
Copy link
Contributor Author

shana commented Apr 20, 2016

Should we always set the padding on the container control, so all views get it automatically? Right now we are setting it in each view (like in https://github.com/github/VisualStudio/blob/feature/pr/views/src/GitHub.VisualStudio/UI/Views/PullRequestListView.xaml#L83).

EDIT: nevermind, sometimes we want to extend lines all the way to the edge. So this view needs some padding 😄

Log into -> Sign in to
Sign up -> Create an account
@donokuda
Copy link
Contributor

Updated the logged out view (note: the panel title's color currently doesn't change when switching themes and probably should be fixed in a separate branch)

image

  • Moved content to top
  • Added margin around the content
  • Synced terminology with what we have on GitHub.com (Login vs. Sign in)

Also, do we want to have more information on why the user should login? Would that be worth it?

👍 I copy-pasta'd the text from the Team Explorer but I definitely think the text should be clarified. We could iterate with what we have with something like "Get powerful collaboration, code review, and code management tools for..."

On a side note, it is possible to have the login happen directly in the pane and not as a separate dialog, if we want to do that at some point. Just fyi.

I've thought about this but figured we could use what's already implemented in the extension. However, this might be worth visiting down the road.

@donokuda donokuda assigned shana and unassigned donokuda Apr 20, 2016
@shana
Copy link
Contributor Author

shana commented Apr 25, 2016

/cc @haacked @pmn for 👀 the copy and the style

@shana shana changed the title WIP [Pull Requests] Logged out view [WIP] Pull Requests: Logged out view Apr 25, 2016
@pmn
Copy link

pmn commented Apr 25, 2016

It looks great to me -- ❤️ both the copy and the style.

@haacked
Copy link
Contributor

haacked commented Apr 25, 2016

👍

@shana shana merged commit 67bd228 into feature/pr/views Apr 27, 2016
@shana shana deleted the feature/pr/logged-out-view branch April 27, 2016 16:57
@shana
Copy link
Contributor Author

shana commented Apr 27, 2016

This needs localization, but let's do a full localization pass on the side for the pr branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants