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

Commit 8a514ca

Browse files
shanashana
authored
Merge pull request #352 from github/fixes/351-show-login-error-message
Show login failure message.
2 parents 0962668 + 455e64b commit 8a514ca

File tree

11 files changed

+122
-106
lines changed

11 files changed

+122
-106
lines changed

‎src/GitHub.App/Resources.Designer.cs‎

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/GitHub.App/Resources.resx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@
123123
<data name="CloneTitle" xml:space="preserve">
124124
<value>Clone a {0} Repository</value>
125125
</data>
126+
<data name="CouldNotConnectToGitHub" xml:space="preserve">
127+
<value>Could not connect to github.com</value>
128+
</data>
126129
<data name="CreateGistTitle" xml:space="preserve">
127130
<value>Create a GitHub Gist</value>
128131
</data>
@@ -141,12 +144,18 @@
141144
<data name="EnterpriseUrlValidatorNotAGitHubHost" xml:space="preserve">
142145
<value>Not an Enterprise server. Please enter an Enterprise URL</value>
143146
</data>
147+
<data name="ForgotPasswordLink" xml:space="preserve">
148+
<value>(forgot your password?)</value>
149+
</data>
144150
<data name="LoginFailedForbiddenMessage" xml:space="preserve">
145151
<value>Make sure to use your password and not a Personal Access token to log in.</value>
146152
</data>
147153
<data name="LoginFailedMessage" xml:space="preserve">
148154
<value>Check your username and password, then try again</value>
149155
</data>
156+
<data name="LoginFailedText" xml:space="preserve">
157+
<value>Login failed.</value>
158+
</data>
150159
<data name="LoginTitle" xml:space="preserve">
151160
<value>Connect To GitHub</value>
152161
</data>

‎src/GitHub.App/ViewModels/LoginTabViewModel.cs‎

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics.CodeAnalysis;
23
using System.Globalization;
34
using System.Reactive;
45
using System.Reactive.Linq;
@@ -18,6 +19,7 @@
1819

1920
namespace GitHub.ViewModels
2021
{
22+
[SuppressMessage("Microsoft.Design", "CA1001:TypesThatOwnDisposableFieldsShouldBeDisposable")]
2123
public abstract class LoginTabViewModel : ReactiveObject
2224
{
2325
static readonly Logger log = LogManager.GetCurrentClassLogger();
@@ -47,23 +49,22 @@ protected LoginTabViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrows
4749
log.Info(string.Format(CultureInfo.InvariantCulture, "Error logging into '{0}' as '{1}'", BaseUri, UsernameOrEmail), ex);
4850
if (ex is Octokit.ForbiddenException)
4951
{
50-
ShowLogInFailedError = true;
51-
LoginFailedMessage = Resources.LoginFailedForbiddenMessage;
52+
UserError.Throw(new UserError(Resources.LoginFailedForbiddenMessage));
5253
}
5354
else
5455
{
55-
ShowConnectingToHostFailed = true;
56+
UserError.Throw(new UserError(ex.Message));
5657
}
5758
});
5859

5960
isLoggingIn = Login.IsExecuting.ToProperty(this, x => x.IsLoggingIn);
6061

6162
Reset = ReactiveCommand.CreateAsyncTask(_ => Clear());
6263

63-
NavigateForgotPassword = ReactiveCommand.CreateAsyncObservable(_ =>
64+
NavigateForgotPassword = new RecoveryCommand(Resources.ForgotPasswordLink, _ =>
6465
{
6566
browser.OpenUrl(new Uri(BaseUri, GitHubUrls.ForgotPasswordPath));
66-
return Observable.Return(Unit.Default);
67+
return RecoveryOptionResult.RetryOperation;
6768
});
6869

6970
SignUp = ReactiveCommand.CreateAsyncObservable(_ =>
@@ -78,14 +79,7 @@ protected LoginTabViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrows
7879

7980
public IReactiveCommand<AuthenticationResult> Login { get; }
8081
public IReactiveCommand<Unit> Reset { get; }
81-
public IReactiveCommand<Unit> NavigateForgotPassword { get; }
82-
83-
string loginFailedMessage = Resources.LoginFailedMessage;
84-
public string LoginFailedMessage
85-
{
86-
get { return loginFailedMessage; }
87-
set { this.RaiseAndSetIfChanged(ref loginFailedMessage, value); }
88-
}
82+
public IRecoveryCommand NavigateForgotPassword { get; }
8983

9084
string usernameOrEmail;
9185
[AllowNull]
@@ -133,29 +127,12 @@ public bool CanLogin
133127
get { return canLogin.Value; }
134128
}
135129

136-
bool showLogInFailedError;
137-
public bool ShowLogInFailedError
138-
{
139-
get { return showLogInFailedError; }
140-
protected set { this.RaiseAndSetIfChanged(ref showLogInFailedError, value); }
141-
}
142-
143-
bool showConnectingToHostFailed;
144-
public bool ShowConnectingToHostFailed
145-
{
146-
get { return showConnectingToHostFailed; }
147-
set { this.RaiseAndSetIfChanged(ref showConnectingToHostFailed, value); }
148-
}
149-
150130
protected abstract IObservable<AuthenticationResult> LogIn(object args);
151131

152132
protected IObservable<AuthenticationResult> LogInToHost(HostAddress hostAddress)
153133
{
154134
return Observable.Defer(() =>
155135
{
156-
ShowLogInFailedError = false;
157-
ShowConnectingToHostFailed = false;
158-
159136
return hostAddress != null ?
160137
RepositoryHosts.LogIn(hostAddress, UsernameOrEmail, Password)
161138
: Observable.Return(AuthenticationResult.CredentialFailure);
@@ -165,12 +142,15 @@ protected IObservable<AuthenticationResult> LogInToHost(HostAddress hostAddress)
165142
switch (authResult)
166143
{
167144
case AuthenticationResult.CredentialFailure:
168-
ShowLogInFailedError = true;
145+
UserError.Throw(new UserError(
146+
Resources.LoginFailedText,
147+
Resources.LoginFailedMessage,
148+
new[] { NavigateForgotPassword }));
169149
break;
170150
case AuthenticationResult.VerificationFailure:
171151
break;
172152
case AuthenticationResult.EnterpriseServerNotFound:
173-
ShowConnectingToHostFailed = true;
153+
UserError.Throw(new UserError(Resources.CouldNotConnectToGitHub));
174154
break;
175155
}
176156
})
@@ -201,8 +181,6 @@ async Task Clear()
201181
await UsernameOrEmailValidator.ResetAsync();
202182
await PasswordValidator.ResetAsync();
203183
await ResetValidation();
204-
205-
ShowLogInFailedError = false;
206184
}
207185

208186
protected virtual Task ResetValidation()

‎src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ protected override async Task ResetValidation()
6969
{
7070
EnterpriseUrl = null;
7171
await EnterpriseUrlValidator.ResetAsync();
72-
ShowConnectingToHostFailed = false;
7372
}
7473
}
7574
}

‎src/GitHub.Exports.Reactive/ViewModels/ILoginToHostViewModel.cs‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,6 @@ public interface ILoginToHostViewModel
5353
/// </summary>
5454
bool IsLoggingIn { get; }
5555

56-
/// <summary>
57-
/// Gets a value indicating whether to show log an error
58-
/// message due to a failed log in.
59-
/// </summary>
60-
bool ShowLogInFailedError { get; }
61-
62-
/// <summary>
63-
/// The message to show if login failed.
64-
/// </summary>
65-
string LoginFailedMessage { get; }
66-
6756
/// <summary>
6857
/// Gets a command which, when invoked, resets all properties
6958
/// and validators.
@@ -74,12 +63,6 @@ public interface ILoginToHostViewModel
7463
/// Gets a command which, when invoked, directs the user to
7564
/// a GitHub.com lost password flow.
7665
/// </summary>
77-
IReactiveCommand<Unit> NavigateForgotPassword { get; }
78-
79-
/// <summary>
80-
/// Gets a value indicating whether to show an error message
81-
/// due to being unable to connect to the host.
82-
/// </summary>
83-
bool ShowConnectingToHostFailed { get; }
66+
IRecoveryCommand NavigateForgotPassword { get; }
8467
}
8568
}

‎src/GitHub.UI.Reactive/Assets/Controls/Validation/UserErrorMessages.xaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
<Grid.ColumnDefinitions>
1717
<ColumnDefinition Width="Auto" />
1818
<ColumnDefinition Width="*"/>
19+
<ColumnDefinition Width="Auto" />
1920
</Grid.ColumnDefinitions>
2021

2122
<Viewbox
@@ -27,6 +28,7 @@
2728
Height="16">
2829
<ui:OcticonPath x:Name="icon" Icon="{TemplateBinding Icon}" Height="16" Fill="{TemplateBinding Fill}" />
2930
</Viewbox>
31+
3032
<ContentControl Grid.Column="1" Content="{Binding}">
3133
<ContentControl.Resources>
3234
<DataTemplate DataType="{x:Type rxui:UserError}">
@@ -41,6 +43,25 @@
4143
</DataTemplate>
4244
</ContentControl.Resources>
4345
</ContentControl>
46+
47+
<ItemsControl Grid.Column="2"
48+
ItemsSource="{Binding RecoveryOptions}"
49+
VerticalAlignment="Center">
50+
<ItemsControl.ItemsPanel>
51+
<ItemsPanelTemplate>
52+
<StackPanel Orientation="Horizontal"/>
53+
</ItemsPanelTemplate>
54+
</ItemsControl.ItemsPanel>
55+
<ItemsControl.ItemTemplate>
56+
<DataTemplate>
57+
<TextBlock Margin="4,0,0,0">
58+
<Hyperlink Command="{Binding}">
59+
<TextBlock Text="{Binding CommandName}"/>
60+
</Hyperlink>
61+
</TextBlock>
62+
</DataTemplate>
63+
</ItemsControl.ItemTemplate>
64+
</ItemsControl>
4465
</Grid>
4566

4667
<ControlTemplate.Resources>

‎src/GitHub.UI.Reactive/Controls/Validation/UserErrorMessages.cs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public UserErrorMessages()
4646
};
4747
}
4848

49-
public static readonly DependencyProperty IconMarginProperty = DependencyProperty.Register("IconMargin", typeof(Thickness), typeof(UserErrorMessages), new PropertyMetadata(new Thickness(0,10,7,0)));
49+
public static readonly DependencyProperty IconMarginProperty = DependencyProperty.Register("IconMargin", typeof(Thickness), typeof(UserErrorMessages), new PropertyMetadata(new Thickness(0,0,8,0)));
5050
public Thickness IconMargin
5151
{
5252
[return: AllowNull]
@@ -106,18 +106,14 @@ public UserError UserError
106106
Justification = "We're registering a handler for a type so this is appropriate.")]
107107
public IDisposable RegisterHandler<TUserError>(IObservable<bool> clearWhen) where TUserError : UserError
108108
{
109-
if (IsVisible)
109+
return UserError.RegisterHandler<TUserError>(userError =>
110110
{
111-
return UserError.RegisterHandler<TUserError>(userError =>
112-
{
113-
UserError = userError;
114-
return clearWhen
115-
.Skip(1)
116-
.Do(_ => UserError = null)
117-
.Select(x => RecoveryOptionResult.CancelOperation);
118-
});
119-
}
120-
return Disposable.Empty;
111+
UserError = userError;
112+
return clearWhen
113+
.Skip(1)
114+
.Do(_ => UserError = null)
115+
.Select(x => RecoveryOptionResult.CancelOperation);
116+
});
121117
}
122118
}
123119
}

‎src/GitHub.VisualStudio/UI/Views/Controls/LoginControl.xaml‎

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
x:Name="hostTabControl"
5454
Margin="30,0"
5555
Style="{StaticResource LightModalViewTabControl}"
56+
SelectionChanged="hostTabControl_SelectionChanged"
5657
FocusManager.IsFocusScope="True"
5758
FocusVisualStyle="{x:Null}"
5859
helpers:AccessKeysManagerScoping.IsEnabled="True">
@@ -97,19 +98,8 @@
9798

9899
<ui:SecurePasswordBox x:Name="dotComPassword" PromptText="{x:Static prop:Resources.PasswordPrompt}" />
99100

100-
<uirx:ErrorMessageDisplay
101-
x:Name="dotComLoginFailedMessage"
102-
Message="{x:Static prop:Resources.LoginFailedMessage}">
103-
<TextBlock TextWrapping="Wrap" Text="{x:Static prop:Resources.LoginFailedText}">
104-
<Hyperlink x:Name="dotComForgotPasswordLink"><TextBlock Text="{x:Static prop:Resources.ForgotPasswordLink}"/></Hyperlink>
105-
</TextBlock>
106-
</uirx:ErrorMessageDisplay>
107-
108-
<uirx:ErrorMessageDisplay
109-
x:Name="dotComConnectionFailedMessage"
110-
Message="{x:Static prop:Resources.dotComConnectionFailedMessageMessage}">
111-
<TextBlock TextWrapping="Wrap" Text="{x:Static prop:Resources.couldNotConnectToGitHubText}"/>
112-
</uirx:ErrorMessageDisplay>
101+
<uirx:UserErrorMessages x:Name="dotComErrorMessage" Margin="0,10">
102+
</uirx:UserErrorMessages>
113103

114104
<uirx:ValidationMessage
115105
x:Name="dotComUserNameOrEmailValidationMessage"
@@ -162,19 +152,8 @@
162152
ValidatesControl="{Binding ElementName=enterpriseUrl}"
163153
TextChangeThrottle="1.0"/>
164154

165-
<uirx:ErrorMessageDisplay
166-
x:Name="enterpriseLoginFailedMessage"
167-
Message="{x:Static prop:Resources.LoginFailedMessage}">
168-
<TextBlock TextWrapping="Wrap" Text="{x:Static prop:Resources.LoginFailedText}">
169-
<Hyperlink x:Name="enterpriseForgotPasswordLink"><TextBlock Text="{x:Static prop:Resources.ForgotPasswordLink}"/></Hyperlink>
170-
</TextBlock>
171-
</uirx:ErrorMessageDisplay>
172-
173-
<uirx:ErrorMessageDisplay
174-
x:Name="enterpriseConnectingFailedMessage"
175-
Message="{x:Static prop:Resources.enterpriseConnectingFailedMessage}">
176-
<TextBlock TextWrapping="Wrap" Text="{x:Static prop:Resources.couldNotConnectToTheServerText}"/>
177-
</uirx:ErrorMessageDisplay>
155+
<uirx:UserErrorMessages x:Name="enterpriseErrorMessage" Margin="0,10">
156+
</uirx:UserErrorMessages>
178157
</StackPanel>
179158
</DockPanel>
180159
</TabItem>

0 commit comments

Comments
 (0)