very barebones support for warning users when rooms contain unknown devices #635
Conversation
|
Stuff to be done here either before it lands or shortly afterwards:
Rich: i hacked on this to try to unstick it, especially given the ongoing WA drama, and given it's the only technical insecurity that we know of in the impl. I'm unlikely to be able to work sensibly on it for the next week thanks to skiing, but if you could take a look it would be hugely appreciated. |
| @@ -0,0 +1,73 @@ | |||
| /* | |||
| Copyright 2015, 2016 OpenMarket Ltd | |||
| @@ -44,7 +44,7 @@ module.exports = React.createClass({ | |||
|
|
|||
| var cancelButton; | |||
| if (this.props.onCancelClick) { | |||
| cancelButton = <div className="mx_RoomHeader_cancelButton" onClick={this.props.onCancelClick}><img src="img/cancel.svg" width="18" height="18" alt="Cancel"/> </div>; | |||
| cancelButton = <div className="mx_RoomHeader_cancelButton" onClick={this.props.onCancelClick}><img src="img/cancel.svg" className="mx_filterFlipColor" width="18" height="18" alt="Cancel"/> </div>; | |||
There was a problem hiding this comment.
oops; random trivial dark theme fix that snuck in. should have gone straight to develop probably.
There was a problem hiding this comment.
ok, factored out to #651 along with a fix that removes some c&p and therefore fixes it for the room settings too...
| limitations under the License. | ||
| */ | ||
|
|
||
| var React = require("react"); |
There was a problem hiding this comment.
would be nice to use import rather than require
|
|
||
| var React = require("react"); | ||
| var sdk = require('../../../index'); | ||
| var MatrixClientPeg = require("../../../MatrixClientPeg"); |
There was a problem hiding this comment.
this appears to be unused. And if it wasn't, I'd be asking you to pass in the matrixClient in a property, rather than adding more references to MatrixClientPeg.
| <h4>We strongly recommend you verify them before continuing.</h4> | ||
| <p>Unknown devices: | ||
| <ul>{ | ||
| Object.keys(this.props.devices).map(userId=>{ |
There was a problem hiding this comment.
could do with factoring this out to a separate function
This was an unrelated change which is being done on another branch anyway
hopefully make this a bit more readable, and use our new BaseDialog.
…matrix-react-sdk into matthew/warn-unknown-devices
|
I reworded the warning somewhat: 2c7b3d9. Modulo element-hq/element-web#3018, which I will look at now, I think this is good to merge, and we can see how it feels and go on to arguing about the follow-ups. WDYT, @ara4n ? |
|
rewrite wfm, as does landing it. |
|
FTR, I gave up trying to fix element-hq/element-web#3018, and am unsure whether we want to land this without it. |
| @@ -177,7 +177,7 @@ class ModalManager { | |||
|
|
|||
| var modal = this._modals[0]; | |||
| var dialog = ( | |||
| <div className={"mx_Dialog_wrapper " + modal.className}> | |||
| <div className={"mx_Dialog_wrapper " + (modal.className ? modal.className : '') }> | |||
There was a problem hiding this comment.
don't we have https://github.com/JedWatson/classnames for this? fine, anyway
There was a problem hiding this comment.
we do. but it's arguably slightly overkill for something trivial like this. plus this was just fixing someone else's bug.
|
The CSS for this is in element-hq/element-web@b137c82. |
Hopefully a step in the right direction towards fixing element-hq/element-web#2143