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

Clean up of Sign In and Sign Up Pages #690

Merged
merged 6 commits into from
Nov 12, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions examples/vue/src/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ body {
align-items: center;
}

.amplify-divider {
display: block !important; /* because of all: unset */
margin: 1rem 0 !important; /* margin doesn't work in React for divider */
}

.amplify-placeholder {
display: block;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
>
<div
class="amplify-flex federated-sign-in-button-row"
style="flex-direction: row; justify-content: center"
style="flex-direction: row; justify-content: center; align-items: center"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixed an issue where the federated icons were slightly above the text on the line

>
<ng-content></ng-content>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
<div
class="amplify-flex federated-sign-in-container"
style="flex-direction: column"
style="flex-direction: column; padding: 0 0 1rem 0"
*ngIf="shouldShowFederatedSignIn"
data-orientation="horizontal"
data-size="small"
>
<hr
class="amplify-divider"
aria-orientation="horizontal"
data-size="small"
style="margin: '1rem 0'"
/>

<amplify-federated-sign-in-button
*ngIf="includeAmazon"
[text]="signInAmazonText"
Expand Down Expand Up @@ -108,4 +101,13 @@
{{ signInGoogleText }}
</p>
</amplify-federated-sign-in-button>

<div class="amplify-flex" data-or-container>
<div data-or-line>or</div>
<hr
class="amplify-divider"
aria-orientation="horizontal"
data-size="small"
/>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,17 @@ export class AmplifyFederatedSignInComponent implements OnInit {
public shouldShowFederatedSignIn = false;

// translated texts
public signInAmazonText = translate('Sign In with Amazon');
public signInAppleText = translate('Sign In with Apple');
public signInFacebookText = translate('Sign In with Facebook');
public signInGoogleText = translate('Sign In with Google');
public signInAmazonText: String;
public signInAppleText: String;
public signInFacebookText: String;
public signInGoogleText: String;
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: we should use string instead of String here:

Suggested change
public signInAmazonText: String;
public signInAppleText: String;
public signInFacebookText: String;
public signInGoogleText: String;
public signInAmazonText: string;
public signInAppleText: string;
public signInFacebookText: string;
public signInGoogleText: string;

https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#number-string-boolean-symbol-and-object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops yup, I'll fix that.


constructor(private authenticator: AuthenticatorService) {}

ngOnInit(): void {
const { socialProviders } = this.authenticator.context?.config;

this.setFederatedTexts();
this.includeAmazon = socialProviders?.includes('amazon');
this.includeApple = socialProviders?.includes('apple');
this.includeGoogle = socialProviders?.includes('google');
Expand All @@ -37,4 +38,21 @@ export class AmplifyFederatedSignInComponent implements OnInit {
this.includeFacebook ||
this.includeGoogle;
}

private setFederatedTexts() {
const { route } = this.authenticator;
const federatedText = route === 'signUp' ? 'Up' : 'In';
this.signInAmazonText = translate<string>(
`Sign ${federatedText} with Amazon`
);
this.signInAppleText = translate<string>(
`Sign ${federatedText} with Apple`
);
this.signInFacebookText = translate<string>(
`Sign ${federatedText} with Facebook`
);
this.signInGoogleText = translate<string>(
`Sign ${federatedText} with Google`
);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
<div data-amplify-container>
<form data-amplify-form (submit)="onSubmit($event)" (input)="onInput($event)">
<amplify-federated-sign-in></amplify-federated-sign-in>
<fieldset
class="amplify-flex"
style="flex-direction: column"
data-amplify-fieldset
[disabled]="authenticator.isPending"
>
<amplify-slot name="sign-in-header">
<h3 class="amplify-heading">{{ this.headerText }}</h3>
</amplify-slot>
<amplify-slot name="sign-in-header"></amplify-slot>
<amplify-user-name-alias></amplify-user-name-alias>
<amplify-form-field
data-amplify-password
Expand All @@ -32,7 +31,6 @@ <h3 class="amplify-heading">{{ this.headerText }}</h3>
{{ authenticator.error }}
</amplify-error>
</fieldset>
<amplify-federated-sign-in></amplify-federated-sign-in>
<amplify-slot name="sign-in-footer"> </amplify-slot>
</form>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { translate } from '@aws-amplify/ui';
})
export class AmplifySignInComponent {
@HostBinding('attr.data-amplify-authenticator-signin') dataAttr = '';
@Input() public headerText = translate('Sign in to your account');

// translated phrases
public forgotPasswordText = translate('Forgot your password? ');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<form data-amplify-form (submit)="onSubmit($event)" (input)="onInput($event)">
<amplify-federated-sign-in></amplify-federated-sign-in>
<div class="amplify-flex" style="flex-direction: column">
<amplify-slot name="sign-up-header">
<h3 class="amplify-heading">{{ this.headerText }}</h3>
</amplify-slot>
<amplify-slot name="sign-up-header"></amplify-slot>
<div class="amplify-flex" style="flex-direction: column">
<amplify-slot name="sign-up-form-fields" [context]="context">
<amplify-sign-up-form-fields></amplify-sign-up-form-fields>
Expand All @@ -26,7 +25,6 @@ <h3 class="amplify-heading">{{ this.headerText }}</h3>
{{ createAccountText }}
</button>
</amplify-slot>
<amplify-federated-sign-in></amplify-federated-sign-in>
<amplify-slot name="sign-up-footer"> </amplify-slot>
</div>
</form>
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import { translate } from '@aws-amplify/ui';
templateUrl: './amplify-sign-up.component.html',
})
export class AmplifySignUpComponent {
@Input() headerText = translate('Create a new account');

@HostBinding('attr.data-amplify-authenticator-signup') dataAttr = '';

// translated texts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ Feature: Internationalization (I18n)

@angular @react @vue
Scenario: Authenticator reflects `I18n.setLanguage('ja')`
Then the "Create a new account" header is in "ja"
And the "Email" input is in "ja"
And the "Password" input is in "ja"
And the "Confirm Password" input is in "ja"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Feature: Sign In with SMS MFA
And I type my password
And I click the "Sign in" button
And I click the "Back to Sign In" button
Then I see "Sign in to your account"
Then I see "Sign in"

@angular @react @vue
Scenario: Incorrect SMS code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Feature: Sign In with TOTP MFA
And I type my password
And I click the "Sign in" button
And I click the "Back to Sign In" button
Then I see "Sign in to your account"
Then I see "Sign in"

@angular @react @vue
Scenario: Invalid TOTP code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Feature: Sign In with Email
And I click the "Sign in" button
Then I see "Sign out"
And I click the "Sign out" button
Then I see "Sign in to your account"
Then I see "Sign in"



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Feature: Sign In with Phone Number
And I click the "Sign in" button
Then I see "Sign out"
And I click the "Sign out" button
Then I see "Sign in to your account"
Then I see "Sign in"


# FORCE_CHANGE_PASSWORD tests are skipped as the temporary passwords used for these
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Feature: Sign In with Username
When I reload the page
Then I see "Sign out"
And I click the "Sign out" button
Then I see "Sign in to your account"
Then I see "Sign in"

# FORCE_CHANGE_PASSWORD tests are skipped as the temporary passwords used for these
# test accounts will expire in Cognito.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Feature: withAuthenticator

@react
Scenario: Application is wrapped with Authenticator
Then I see "Sign in to your account"
Then I see "Sign in"

@react
Scenario: Application renders when signed in
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
import { FederatedIdentityProviders, translate } from '@aws-amplify/ui';

import { useAuthenticator } from '..';
import { Divider, Flex } from '../../..';
import { Divider, Flex, View } from '../../..';
import { FederatedSignInButton } from './FederatedSignInButtons';

export function FederatedSignIn() {
const { _state } = useAuthenticator();
const { _state, route } = useAuthenticator();
const { socialProviders = [] } = _state.context.config;

if (socialProviders.length === 0) {
return null;
}

return (
<Flex direction="column" className="federated-sign-in-container">
<Divider size="small" />
const federatedText = route === 'signUp' ? 'Up' : 'In';

return (
<Flex
direction="column"
padding={`0 0 1rem 0`}
className="federated-sign-in-container"
>
{socialProviders.map((provider) => {
switch (provider) {
case 'amazon':
Expand All @@ -24,7 +28,7 @@ export function FederatedSignIn() {
icon="amazon"
key={provider}
provider={FederatedIdentityProviders.Amazon}
text={translate('Sign In with Amazon')}
text={translate<string>(`Sign ${federatedText} with Amazon`)}
/>
);
case 'apple':
Expand All @@ -33,7 +37,7 @@ export function FederatedSignIn() {
icon="apple"
key={provider}
provider={FederatedIdentityProviders.Apple}
text={translate('Sign In with Apple')}
text={translate<string>(`Sign ${federatedText} with Apple`)}
/>
);
case 'facebook':
Expand All @@ -42,7 +46,7 @@ export function FederatedSignIn() {
icon="facebook"
key={provider}
provider={FederatedIdentityProviders.Facebook}
text={translate('Sign In with Facebook')}
text={translate<string>(`Sign ${federatedText} with Facebook`)}
/>
);
case 'google':
Expand All @@ -51,7 +55,7 @@ export function FederatedSignIn() {
icon="google"
key={provider}
provider={FederatedIdentityProviders.Google}
text={translate('Sign In with Google')}
text={translate<string>(`Sign ${federatedText} with Google`)}
/>
);
default:
Expand All @@ -60,6 +64,11 @@ export function FederatedSignIn() {
);
}
})}

<Flex data-or-container="">
<View data-or-line>or</View>
<Divider size="small" />
</Flex>
</Flex>
);
}
Loading