From 393e8ff6125de81800da86861bf3b0cfec81c781 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 7 Mar 2016 22:27:35 +0000 Subject: [PATCH 1/3] Revert "Revert "Merge pull request #193 from matrix-org/rav/shouldComponentUpdates"" Put the shouldComponentUpdate methods back in, now that Matthew has stopped demoing. This reverts commit 606fdcb8dff224cf36f5e27ec9a780fb87812a9f. --- src/ObjectUtils.js | 31 ++++++++++++++++++++++ src/components/structures/RoomView.js | 6 +++++ src/components/structures/TimelinePanel.js | 6 +++++ 3 files changed, 43 insertions(+) diff --git a/src/ObjectUtils.js b/src/ObjectUtils.js index 41dc34cca7..07a16df501 100644 --- a/src/ObjectUtils.js +++ b/src/ObjectUtils.js @@ -77,3 +77,34 @@ module.exports.getKeyValueArrayDiffs = function(before, after) { return results; }; + +/** + * Shallow-compare two objects for equality: each key and value must be + * identical + */ +module.exports.shallowEqual = function(objA, objB) { + if (objA === objB) { + return true; + } + + if (typeof objA !== 'object' || objA === null || + typeof objB !== 'object' || objB === null) { + return false; + } + + var keysA = Object.keys(objA); + var keysB = Object.keys(objB); + + if (keysA.length !== keysB.length) { + return false; + } + + for (var i = 0; i < keysA.length; i++) { + var key = keysA[i]; + if (!objB.hasOwnProperty(key) || objA[key] !== objB[key]) { + return false; + } + } + + return true; +}; diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index e3d60ed83b..fc2190e498 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -38,6 +38,7 @@ var SlashCommands = require("../../SlashCommands"); var dis = require("../../dispatcher"); var Tinter = require("../../Tinter"); var rate_limited_func = require('../../ratelimitedfunc'); +var ObjectUtils = require('../../ObjectUtils'); var DEBUG = false; @@ -164,6 +165,11 @@ module.exports = React.createClass({ } }, + shouldComponentUpdate: function(nextProps, nextState) { + return (!ObjectUtils.shallowEqual(this.props, nextProps) || + !ObjectUtils.shallowEqual(this.state, nextState)); + }, + componentWillUnmount: function() { // set a boolean to say we've been unmounted, which any pending // promises can use to throw away their results. diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index f4747cb026..c320d0b99b 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -24,6 +24,7 @@ var EventTimeline = Matrix.EventTimeline; var sdk = require('../../index'); var MatrixClientPeg = require("../../MatrixClientPeg"); var dis = require("../../dispatcher"); +var ObjectUtils = require('../../ObjectUtils'); var PAGINATE_SIZE = 20; var INITIAL_SIZE = 20; @@ -146,6 +147,11 @@ var TimelinePanel = React.createClass({ } }, + shouldComponentUpdate: function(nextProps, nextState) { + return (!ObjectUtils.shallowEqual(this.props, nextProps) || + !ObjectUtils.shallowEqual(this.state, nextState)); + }, + componentWillUnmount: function() { // set a boolean to say we've been unmounted, which any pending // promises can use to throw away their results. From 6c928f12b2f25dd5b866d23f5022016d3ab1229b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 8 Mar 2016 12:16:34 +0000 Subject: [PATCH 2/3] ForceUpdate the scrollpanel when the aux panel changes size Catch some more cases when we ought to be updating the gemini scroll stuff. --- src/components/structures/RoomView.js | 28 ++++++++++++++------------ src/components/views/rooms/AuxPanel.js | 21 +++++++++++++++---- src/components/views/voip/CallView.js | 8 ++++++-- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index fc2190e498..84ad8f595c 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -991,10 +991,10 @@ module.exports = React.createClass({ // but it's better than the video going missing entirely if (auxPanelMaxHeight < 50) auxPanelMaxHeight = 50; - // we may need to resize the gemini panel after changing the aux panel - // size, so add a callback to onChildResize. - this.setState({auxPanelMaxHeight: auxPanelMaxHeight}, - this.onChildResize); + this.setState({auxPanelMaxHeight: auxPanelMaxHeight}); + + // changing the maxHeight on the auxpanel will trigger a callback go + // onChildResize, so no need to worry about that here. }, onFullscreenClick: function() { @@ -1042,8 +1042,15 @@ module.exports = React.createClass({ // XXX: this is a bit naughty; we should be doing this via props if (show) { this.setState({editingRoomSettings: true}); - var self = this; - setTimeout(function() { self.onResize() }, 0); + } + }, + + // this has to be a proper method rather than an unnamed function, + // otherwise react calls it with null on each update. + _gatherTimelinePanelRef: function(r) { + this.refs.messagePanel = r; + if(r) { + this.updateTint(); } }, @@ -1206,7 +1213,7 @@ module.exports = React.createClass({ draggingFile={this.state.draggingFile} displayConfCallNotification={this.state.displayConfCallNotification} maxHeight={this.state.auxPanelMaxHeight} - onCallViewVideoRezize={this.onChildResize} > + onResize={this.onChildResize} > { aux } ); @@ -1285,12 +1292,7 @@ module.exports = React.createClass({ } var messagePanel = ( - { - this.refs.messagePanel = r; - if(r) { - this.updateTint(); - } - }} +