From 0f52c0a514fa27f472418740e099149793f75f43 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 7 Jan 2016 03:39:00 +0000 Subject: [PATCH] make TintableSvgs responsible for updating their own tints, and stop storing SVG DOM fragments in Tinter to avoid leaking them --- src/Tinter.js | 29 ++++++-------------- src/components/views/elements/TintableSvg.js | 10 +++++++ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/Tinter.js b/src/Tinter.js index 10a103f0f4..f5cd43212c 100644 --- a/src/Tinter.js +++ b/src/Tinter.js @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +var dis = require("./dispatcher"); // FIXME: these vars should be bundled up and attached to // module.exports otherwise this will break when included by both @@ -63,21 +64,11 @@ var cssAttrs = [ "borderColor", ]; -var svgFixups = [ - // { - // node: a SVG node that needs to be fixed up - // attr: name of the attribute to be clobbered, e.g. 'fill' - // index: ordinal of primary, secondary - // } -]; - var svgAttrs = [ "fill", "stroke", ]; -var svgNodes = {}; - var cached = false; function calcCssFixups() { @@ -102,11 +93,14 @@ function calcCssFixups() { } } -function calcSvgFixups(nodes) { - var svgs = nodes || document.getElementsByClassName("mx_TintableSvg"); +function calcSvgFixups(svgs) { + // go through manually fixing up SVG colours. + // we could do this by stylesheets, but keeping the stylesheets + // updated would be a PITA, so just brute-force search for the + // key colour; cache the element and apply. + var fixups = []; for (var i = 0; i < svgs.length; i++) { - var svgDoc = svgs[i].contentDocument; if (!svgDoc) continue; var tags = svgDoc.getElementsByTagName("*"); @@ -167,7 +161,6 @@ module.exports = { tint: function(primaryColor, secondaryColor, tertiaryColor) { if (!cached) { calcCssFixups(); - svgFixups = calcSvgFixups(); cached = true; } @@ -195,11 +188,8 @@ module.exports = { // go through manually fixing up the stylesheets. applyCssFixups(); - // go through manually fixing up SVG colours. - // we could do this by stylesheets, but keeping the stylesheets - // updated would be a PITA, so just brute-force search for the - // key colour; cache the element and apply. - applySvgFixups(svgFixups); + // tell all the SVGs to go fix themselves up + dis.dispatch({ action: 'tint_update' }); }, tintSvg: function(svg) { @@ -207,7 +197,6 @@ module.exports = { // (although this would result in an even worse flicker as the element redraws) var fixups = calcSvgFixups([ svg ]); if (fixups.length) { - svgFixups = svgFixups.concat(fixups); // XXX: this leaks fixups applySvgFixups(fixups); } }, diff --git a/src/components/views/elements/TintableSvg.js b/src/components/views/elements/TintableSvg.js index 5bd19b85dc..48d0b388d5 100644 --- a/src/components/views/elements/TintableSvg.js +++ b/src/components/views/elements/TintableSvg.js @@ -31,6 +31,10 @@ module.exports = React.createClass({ className: React.PropTypes.string, }, + componentWillMount: function() { + this.dispatcherRef = dis.register(this.onAction); + }, + componentDidMount: function() { // we can't use onLoad on object due to https://github.com/facebook/react/pull/5781 // so handle it with pure DOM instead @@ -39,6 +43,12 @@ module.exports = React.createClass({ componentWillUnmount: function() { ReactDOM.findDOMNode(this).removeEventListener('load', this.onLoad); + dis.unregister(this.dispatcherRef); + }, + + onAction: function(payload) { + if (payload.action !== 'tint_update') return; + Tinter.tintSvg(ReactDOM.findDOMNode(this)); }, onLoad: function(event) {