-
Notifications
You must be signed in to change notification settings - Fork 23
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
Feature/blockstack #30
Conversation
Pull Request Test Coverage Report for Build 68
💛 - Coveralls |
@billxinli do you want me to review it now or should I wait until you're done? |
a3b9f38
to
b5005bf
Compare
- Ejected create-react-app - Added jest coverage - Cleaned up code
b5005bf
to
ef7cbcf
Compare
@JackNeto Could you please review the above? Thanks! |
src/lib/BlockstackService.js
Outdated
@@ -0,0 +1,5 @@ | |||
const blockstack = require('blockstack') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seem unnecessary
import React from 'react' | ||
import Button from 'material-ui/Button' | ||
import { withStyles } from 'material-ui/styles' | ||
import BlockstackService from './../../lib/BlockstackService' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having a BlockstackService
it seems simpler to just import the methods from blockstack js
import {
redirectToSignIn,
handlePendingSignIn,
signUserOut,
isUserSignedIn,
loadUserData,
Person
} from 'blockstack';
This is a weird issue, I am not sure what is causing it, I will have to take a look to see how ES6 imports and module.exports works together. If I do: However, it works if I do Where as if I do:
And I inspect the https://nodejs.org/api/esm.html#esm_interop_with_existing_modules I am not sure if there is something wonky going on with Webpack + Babel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works well for me. I was seeing the problem you mentioned. Make sure you run yarn install
and then restart the server.
Summary
This PR added the following features
This PR modified the following features
Screenshot
How to test it
yarn test
to see the test failing because of coverageClick the login button
to see your information being pulled from blockstackOther Information
I will split PRs into seperate things in the future.