-
-
Notifications
You must be signed in to change notification settings - Fork 833
Conversation
@@ -139,6 +139,10 @@ module.exports = React.createClass({ | |||
return this.props.config.default_is_url || "https://vector.im"; | |||
}, | |||
|
|||
getEnableLabs() { | |||
return this.props.config.enableLabs === undefined ? true : this.props.config.enableLabs; |
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.
wouldn't it make more sense to default to false
?
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.
Well, just having it shown doesn't turn any options on, so I figured leave it on by default, but really happy to do either to be honest.
@@ -359,6 +363,8 @@ module.exports = React.createClass({ | |||
}, | |||
|
|||
_renderLabs: function () { | |||
if (this.props.enableLabs === false) return 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.
!this.props.enableLabs
?
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.
Intention was to make it default to enabled if it wasn't specified, which I'ver clarified in a comment.
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.
But if it's not specified, it will get the value from defaultProps
On 5 August 2016 17:19:32 BST, David Baker notifications@github.com wrote:
@@ -359,6 +363,8 @@ module.exports = React.createClass({
},_renderLabs: function () {
if (this.props.enableLabs === false) return null;
Intention was to make it default to enabled if it wasn't specified,
which I'ver clarified in a comment.
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub:
https://github.com/matrix-org/matrix-react-sdk/pull/393/files/f8d02c1551ff9392be45a0cfed0fcd33d5078048#r73719512
lgtm otherwise |
No description provided.