Conversation
aaf9fbb to
2344fef
Compare
…Button components
2344fef to
5ae3ae9
Compare
|
|
||
| **Note:** The same options that are supported by the `SocialLoginLink` component are also supported by this. | ||
|
|
||
| **Important:** This component relies on [Font Awesome](http://fortawesome.github.io/Font-Awesome/) in order to render icons for the various social providers. So if you want to use this button with icons, you also need to install Font Awesome on your site. |
There was a problem hiding this comment.
Don't forget to add Font Awesome to the example project :)
There was a problem hiding this comment.
It's actually added, didn't you try it with the social-login branch? :)
https://github.com/stormpath/stormpath-express-react-example/tree/social-login/build/fonts
|
In the AngularJS SDK we've also displaying the buttons on the registrations form, but you are only showing it on the login form. And also, the AngularJS SDK have colored buttons, while yours are just white. What's your thoughts on that? |
| <SocialLoginLink providerId='facebook'>Sign in with Facebook</SocialLoginLink> | ||
| ``` | ||
|
|
||
| Renders a link that can be used to sign into a social provider. |
There was a problem hiding this comment.
Renders a link that can be used to sign into a social provider.
The user doesn't sign in to a social provider, so it should probably be more like this:
Renders a link that can be used to sign in using a social provider.
|
Other than my comments this is really nice! 👍 |
|
@timothyej I personally see the social login buttons on the signup page as an anti pattern. But I'll add it for consistency. I personally don't think that the colors add much value, so I just decided to go without styling them in the first version. It can be added in a later version. |
As a user I would never click on Login if I wouldn't already have an account, I would always click on Signup. But in this case I would not be able to signup with e.g. Facebook because that button would only be on the login page. |
2366313 to
eb2d059
Compare
|
@timothyej Comments have been fixed. Please review again :) |
| var providerId = this.props.providerId; | ||
|
|
||
| return ( | ||
| <a {...this.props} href='#' className={this.props.className} onClick={this._onClick.bind(this)} disabled={this.state.disabled}> |
There was a problem hiding this comment.
You probably don't have to set className explicitly since you include all the properties using {...this.props}?
There was a problem hiding this comment.
Good catch. Will fix that! :)
|
I've verified that everything works as expected :) I will merge this once you've fixed my comment. |
|
Sweeeet! :) |
|
@timothyej This is now fixed. |
Add support for social login with the
SocialLoginLinkandSocialLoginButtoncomponents.Note
This depends on stormpath/express-stormpath#374 in order for Facebook to be supported.
How to verify
$ npm run build).$ npm link).social-loginbranch of the React example app.$ npm link react-stormpath).$ npm start)./login.Or login with....Or login with...box.Or login with...box disappears.SocialLoginLinkandSocialLoginButton.Fixes #14.