-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Reader: Add a back button to site and feed streams #332
Conversation
@@ -140,6 +151,7 @@ var FeedStream = React.createClass( { | |||
|
|||
return ( | |||
<FollowingStream { ...this.props } listName={ this.state.title } emptyContent={ emptyContent } > | |||
{ this.props.showBack ? <HeaderCake isCompact={ false } onClick={ this.goBack } /> : null } |
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.
What about writing { this.props.showBack && <HeaderCake /> }
with the null
being implicit?
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.
Makes me slightly nervous because when showBack is falsey, it'll render whatever is in showBack, which may not be null.
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 as-expected with false
, so maybe just enforce the boolean proptype?
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.
+1 for using a ternary. Saw another PR earlier today that had its check value being rendered to the page.
👍 tested in Chrome, FF, IE11, Edge. |
…r-streams Reader: Add a back button to site and feed streams
With the ability to override it.