Skip to content

Added logout button #54

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

Merged
merged 6 commits into from
Jun 22, 2018
Merged

Added logout button #54

merged 6 commits into from
Jun 22, 2018

Conversation

SGarno
Copy link
Contributor

@SGarno SGarno commented Jun 22, 2018

Simple update to the boilerplate which allows returning back to the main page after a login name has been locally stored.

@SGarno
Copy link
Contributor Author

SGarno commented Jun 22, 2018

Rats! Forgot to put in the tests. Added in this latest push.

@pronebird
Copy link
Collaborator

pronebird commented Jun 22, 2018

Great work! 🎉

Travis errors because of read-config package that does not properly transpile for node 6.

Since tests run in electron-mocha which spins off Electron internally, it really makes no point to test on different versions of Node.js.

Would you mind updating .travis.yml and removing all node versions except node 8?

node_js:
  - '8'
-  - '7'
-  - '6'

});
}


Copy link
Owner

Choose a reason for hiding this comment

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

Can we remove the additional newline?





Copy link
Owner

Choose a reason for hiding this comment

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

Here too

Copy link
Collaborator

@pronebird pronebird Jun 22, 2018

Choose a reason for hiding this comment

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

Good spot. Btw I've been using prettier for a while now and we never ever had to bother about code style again. May make sense to plug-in prettier and add yarn format to re-format all the code, that also removes trailing newlines etc..

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah definitely agreed! I use prettier in all my projects now. I think it's definitely worthwhile to add.

@SGarno
Copy link
Contributor Author

SGarno commented Jun 22, 2018

Sorry about the blank lines...I have been using Beautify (but it is manual) and am looking at prettier now. I have it installed in VSCode and in the project as a part of the pre-commit.

However, in VSCode it is messing up with JSX. Check out how it formatted the render:

render() {
return ( < div >
<
h2 > Logged in as {
this.props.user.username
} < /h2> Logout < /div>
);
}

Which of the 10 million Prettier plug-ins are you using?

I haven't looked at the pre-commit to see if that is working yet.

@pronebird
Copy link
Collaborator

pronebird commented Jun 22, 2018

I use prettier from terminal to be fair, i.e:

yarn add prettier --global

then run as

prettier --write 'app/**/*.js' 'test/**/*.js'

My current config .prettierrc.yml:

# .prettierrc.yml
# see: https://prettier.io/docs/en/options.html
printWidth: 100
tabWidth: 2
semi: true
useTabs: false
singleQuote: true
trailingComma: all
bracketSpacing: true
jsxBracketSameLine: true
arrowParens: always
proseWrap: always

@SGarno
Copy link
Contributor Author

SGarno commented Jun 22, 2018

Crap! Nevermind....Beautify was still active and they were conflicting......ok.....I will get a new commit here shortly.

@jschr
Copy link
Owner

jschr commented Jun 22, 2018

@BlackConure don't worry about adding prettier in this PR. We've opened up an issue which we'll track separately: #56

Just remove the lines manually and update the travis config per pronebirds suggestion and we're all set!

@pronebird
Copy link
Collaborator

pronebird commented Jun 22, 2018

What @jschr said, no point to bother with the exact style, we'll reformat everything later.

@SGarno
Copy link
Contributor Author

SGarno commented Jun 22, 2018

Ok will do...few mins.

@SGarno
Copy link
Contributor Author

SGarno commented Jun 22, 2018

Sorry....looks like prettier default was double quotes and now I just found another place it needs to be corrected. Re-submitting.

@SGarno
Copy link
Contributor Author

SGarno commented Jun 22, 2018

FYI...if you are using VSCode, you might want to look at the add-in Prettier Now instead of Esben Petersen's version. It seems to be better about JSX, and it also didn't break long lines (nor put double quotes in as a default).

Copy link
Collaborator

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Awesome! 🎉

@pronebird
Copy link
Collaborator

pronebird commented Jun 22, 2018

Yeah, prettier uses double quote in JSX and I think I read that JSX has some issues with single quotes and that it's impossible to escape ' within a string enclosed into single quotes or something like that. We'll probably relax ESLint settings when prettier is added to this boilerplate.

@jschr jschr merged commit 36acc11 into jschr:master Jun 22, 2018
@jschr
Copy link
Owner

jschr commented Jun 22, 2018

Thanks @BlackConure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants