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

Display logged user #57

Merged
merged 8 commits into from
Sep 13, 2017
Merged

Display logged user #57

merged 8 commits into from
Sep 13, 2017

Conversation

ivanzusko
Copy link
Contributor

No description provided.

@@ -0,0 +1,6 @@
export const parseJwt = token => {
const base64Url = token.split('.')[1];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a library to parse JWT. Maybe this one https://github.com/auth0/node-jsonwebtoken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, @italolelis

@@ -7,7 +8,12 @@ import './Header.css';

const b = block('j-header');

const Header = ({ logged }) => {
const propTypes = {
logged: PropTypes.bool.isRequired,
Copy link
Contributor

Choose a reason for hiding this comment

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

if you already have the user wouldn't that already imply that it is logged in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually yes, @Vxplus , totally agree!

@@ -18,6 +24,12 @@ const Header = ({ logged }) => {
<Nav />
</div>
<div className={b('col', { right: true })}>
{
user &&
<span className={b('user')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe user-name or display-name would be more specific for this user sounds like a container imo.

const Header = ({ logged }) => {
const propTypes = {
logged: PropTypes.bool.isRequired,
user: PropTypes.string.isRequired,
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe username fits this more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe in future, there will appear some additional info, so I will change user<String> to user<Object> and fill it with all necessary info. What do you think, @Vxplus ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine by me.

@ivanzusko ivanzusko merged commit cdb8e0e into master Sep 13, 2017
@ivanzusko ivanzusko deleted the feature/logged-user branch September 13, 2017 16:58
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.

None yet

3 participants