Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSwitch to AuthView on Publish API Failure #300
Conversation
Yup, I'm
I can push up an example branch if that helps! |
|
Cool. Thanks for the draft. |
dc579f2
to
90d1f0c
… we get from the api
…re-switch-auth-view
…counts
…ting specific views
…hed username
…e-switch-auth-view
…e-switch-auth-view
…e-switch-auth-view
Use default padding settings
|
Added back the header styles:
|
…e-switch-auth-view
| var usernameMismatchException = exception as TokenUsernameMismatchException; | ||
| if (usernameMismatchException != null) | ||
| { | ||
| message = "Your credentials need to be refreshed"; |
This comment has been minimized.
This comment has been minimized.
shana
Oct 19, 2017
Contributor
Been thinking about this some more and talking to some people about what messages should say, and we should probably explain a bit more why we're doing this.
How about:
We've detected that your stored credentials are out of sync with your current user. This can happen if you have signed in to git outside of Unity. Sign in again to refresh your credentials.
This comment has been minimized.
This comment has been minimized.
|
|
||
| if (usernameMismatchException == null && keychainEmptyException == null) | ||
| { | ||
| message = "There was an error validating your account"; |
This comment has been minimized.
This comment has been minimized.
shana
Oct 19, 2017
Contributor
All of these messages should be in const fields somewhere so we can localize them later
|
|
||
| if (usernameMismatchException == null && keychainEmptyException == null) | ||
| { | ||
| message = "There was an error validating your account"; |
This comment has been minimized.
This comment has been minimized.
shana
Oct 19, 2017
Contributor
I feel like this message is a bit unhelpful, especially given that it's the first thing they see right after clicking the button. When does this happen and is this our fault or the users'? If it's our fault, then we should followup with "Please sign in again" to tell them how to proceed here.
This comment has been minimized.
This comment has been minimized.
StanleyGoldman
Oct 19, 2017
Author
Member
It actually hasn't happened to me yet, so it would probably be best to leave the original exception message there instead.
|
Here's where this PR is at right now. It's ready for some feedback: Sign in view with warning:
2FA view (is there a way to suppress that error if this is the first time viewing this screen?)
I also noticed when you hit "back" from the 2FA, it will still persist the error message even though it's not relevant to the screen before. Is there a way to prevent that from happening?
|
…t-objects Preventing the exposure of Octokit objects to the codebase
|
I like this UI a lot, those message box styles rock!
We probably need to call Repaint/Redraw on the window to refesh the UI here I wonder how we handle the Esc key. We should probably close the window and cancel the whole thing. |
|
|
||
| if (shouldProceed) | ||
| { | ||
| //Proceed as current user |
This comment has been minimized.
This comment has been minimized.
| } | ||
| else | ||
| { | ||
| //Logout current user and try again |
This comment has been minimized.
This comment has been minimized.
|
Actually... the more I look at it this code will never run, Unity/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs Lines 126 to 147 in b108e06 |
|
omgomgomgomgomg this looks so awesome! |
|
Fixes #414 |
StanleyGoldman commentedSep 11, 2017
•
edited by shana
Currently our only exposure to the API is
PublishView. In the event of an authentication failure we need to switch thePublishViewto theAuthenticationView-If there is no authentication present
Depends on: