-
Notifications
You must be signed in to change notification settings - Fork 8
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
[BT-151] AppHeader #42
Changes from 6 commits
126879e
9b9c261
51ae2dd
b99c099
b6141a9
fd351f1
6b2124a
53b93aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
|
||
import React, { | ||
Component, | ||
PropTypes, | ||
} from 'react'; | ||
|
||
import AccountPicture from './AccountPicture.jsx'; | ||
|
||
export default class AccountNav extends Component { | ||
|
||
constructor(props) { | ||
super(props); | ||
this.state = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should move this into In short, I don't think it hurts anything to move this into the props, and it gives us the option of reacting to or acting upon this state globally. |
||
isOpen: false, | ||
}; | ||
} | ||
|
||
onClick() { | ||
this.setState({ | ||
isOpen: !this.state.isOpen, | ||
}); | ||
} | ||
|
||
render() { | ||
const dropdownClass = !this.state.isOpen ? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use const dropdownClass = classNames('accountDropdownArrow', {
'is-account-dropdown-arrow-open': this.props.isOpen,
}); |
||
'dropdownArrow' : | ||
'dropdownArrow--reverse'; | ||
|
||
return ( | ||
<a | ||
className="accountNav" | ||
onClick={this.onClick.bind(this)} | ||
> | ||
<AccountPicture | ||
url={this.props.pictureUrl} | ||
title={this.props.email} | ||
/> | ||
<span className="accountNav__email"> | ||
{this.props.email} | ||
</span> | ||
<span className={dropdownClass}></span> | ||
</a> | ||
); | ||
} | ||
|
||
} | ||
|
||
AccountNav.propTypes = { | ||
email: PropTypes.string.isRequired, | ||
pictureUrl: AccountPicture.propTypes.url, | ||
}; | ||
|
||
export default AccountNav; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
|
||
import React, { | ||
Component, | ||
PropTypes, | ||
} from 'react'; | ||
|
||
export default class AccountPicture extends Component { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could be made into a pure functional component. |
||
|
||
constructor(props) { | ||
super(props); | ||
} | ||
|
||
render() { | ||
const picture = this.props.url ? <img src={this.props.url} /> : null; | ||
|
||
return ( | ||
<span className="icon glyphicons-user accountPicture"> | ||
{picture} | ||
</span> | ||
); | ||
} | ||
|
||
} | ||
|
||
AccountPicture.propTypes = { | ||
url: PropTypes.string, | ||
title: PropTypes.string, | ||
}; | ||
|
||
export default AccountPicture; |
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.
I think this could be made into a pure functional component. It will make the code shorter and reduce memory usage (a miniscule amount). Unless we anticipate needing to optimize this component with lifecycle hooks, I think it would be good to make it a pure functional component.