From b3e6cbfddd7115bfd9aa1d44fd6535bba3ec2883 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 21 Feb 2019 18:51:18 +0100 Subject: [PATCH 01/34] restore scroll state when timeline resizes using ResizeObserver (only where supported, polyfill doesn't give good results) --- src/components/structures/ScrollPanel.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index a1a7d08e0b..db19b1d0cc 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -160,6 +160,13 @@ module.exports = React.createClass({ componentDidMount: function() { this.checkScroll(); + + if (typeof ResizeObserver !== "undefined") { + this._timelineSizeObserver = new ResizeObserver(() => { + this._restoreSavedScrollState(); + }); + this._timelineSizeObserver.observe(this.refs.itemlist); + } }, componentDidUpdate: function() { @@ -181,6 +188,10 @@ module.exports = React.createClass({ // // (We could use isMounted(), but facebook have deprecated that.) this.unmounted = true; + if (this._timelineSizeObserver) { + this._timelineSizeObserver.disconnect(); + this._timelineSizeObserver = null; + } }, onScroll: function(ev) { From ecb074862ecfdf8a9c866d2c32438c2efc91111c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 21 Feb 2019 18:51:58 +0100 Subject: [PATCH 02/34] remove fix for old chrome bug --- src/components/structures/ScrollPanel.js | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index db19b1d0cc..55ac753edb 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -199,24 +199,6 @@ module.exports = React.createClass({ debuglog("Scroll event: offset now:", sn.scrollTop, "_lastSetScroll:", this._lastSetScroll); - // Sometimes we see attempts to write to scrollTop essentially being - // ignored. (Or rather, it is successfully written, but on the next - // scroll event, it's been reset again). - // - // This was observed on Chrome 47, when scrolling using the trackpad in OS - // X Yosemite. Can't reproduce on El Capitan. Our theory is that this is - // due to Chrome not being able to cope with the scroll offset being reset - // while a two-finger drag is in progress. - // - // By way of a workaround, we detect this situation and just keep - // resetting scrollTop until we see the scroll node have the right - // value. - if (this._lastSetScroll !== undefined && sn.scrollTop < this._lastSetScroll-200) { - console.log("Working around vector-im/vector-web#528"); - this._restoreSavedScrollState(); - return; - } - // If there weren't enough children to fill the viewport, the scroll we // got might be different to the scroll we wanted; we don't want to // forget what we wanted, so don't overwrite the saved state unless From 03784e586c7416fdc03670bc076ecee6cbb89c37 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 21 Feb 2019 19:28:55 +0100 Subject: [PATCH 03/34] replace getBoundingClientRect() with offset/scrollTop & clientHeight as they are an order of magnitude faster in most browsers, getBoundingClientRect() tends to cause relayout. --- src/components/structures/ScrollPanel.js | 45 ++++++++++++++---------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 55ac753edb..1433baeeb9 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -576,9 +576,10 @@ module.exports = React.createClass({ } const scrollNode = this._getScrollNode(); - const wrapperRect = ReactDOM.findDOMNode(this).getBoundingClientRect(); - const boundingRect = node.getBoundingClientRect(); - const scrollDelta = boundingRect.bottom + pixelOffset - wrapperRect.bottom; + const scrollBottom = scrollNode.scrollTop + scrollNode.clientHeight; + + const nodeBottom = node.offsetTop + node.clientHeight; + const scrollDelta = nodeBottom + pixelOffset - scrollBottom; debuglog("ScrollPanel: scrolling to token '" + scrollToken + "'+" + pixelOffset + " (delta: "+scrollDelta+")"); @@ -595,37 +596,43 @@ module.exports = React.createClass({ return; } + const scrollNode = this._getScrollNode(); + const scrollBottom = scrollNode.scrollTop + scrollNode.clientHeight; + const itemlist = this.refs.itemlist; - const wrapperRect = ReactDOM.findDOMNode(this).getBoundingClientRect(); const messages = itemlist.children; - let newScrollState = null; + let node = null; + let nodeBottom; + // TODO: change this to not use getBoundingClientRect and save the domnode for (let i = messages.length-1; i >= 0; --i) { - const node = messages[i]; - if (!node.dataset.scrollTokens) continue; + if (!messages[i].dataset.scrollTokens) { + continue; + } + node = messages[i]; - const boundingRect = node.getBoundingClientRect(); - newScrollState = { - stuckAtBottom: false, - trackedScrollToken: node.dataset.scrollTokens.split(',')[0], - pixelOffset: wrapperRect.bottom - boundingRect.bottom, - }; + nodeBottom = node.offsetTop + node.clientHeight; // If the bottom of the panel intersects the ClientRect of node, use this node // as the scrollToken. // If this is false for the entire for-loop, we default to the last node // (which is why newScrollState is set on every iteration). - if (boundingRect.top < wrapperRect.bottom) { + if (nodeBottom >= scrollBottom) { // Use this node as the scrollToken break; } } - // This is only false if there were no nodes with `node.dataset.scrollTokens` set. - if (newScrollState) { - this.scrollState = newScrollState; - debuglog("ScrollPanel: saved scroll state", this.scrollState); - } else { + + if (!node) { debuglog("ScrollPanel: unable to save scroll state: found no children in the viewport"); + return; } + + debuglog("ScrollPanel: saved scroll state", this.scrollState); + this.scrollState = { + stuckAtBottom: false, + trackedScrollToken: node.dataset.scrollTokens.split(',')[0], + pixelOffset: scrollBottom - nodeBottom, + }; }, _restoreSavedScrollState: function() { From 41ae618d0ea7a56e9bcf3d88827aaa269f2b3f63 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Feb 2019 13:12:52 +0100 Subject: [PATCH 04/34] only clear min-height on scroll & adding items (componentDidUpdate) before we would clear it as soon as you were 1px away from the bottom of the timeline, which would still create jumping as the whitespace would around 36px. To play it safe, we only clear it after moving 200px from the bottom. Also include "local echo" scroll events, caused by setting scrollTop --- src/components/structures/ScrollPanel.js | 35 ++++++++++++------------ 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 1433baeeb9..154f7dfc12 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -176,10 +176,6 @@ module.exports = React.createClass({ // // This will also re-check the fill state, in case the paginate was inadequate this.checkScroll(); - - if (!this.isAtBottom()) { - this.clearBlockShrinking(); - } }, componentWillUnmount: function() { @@ -204,23 +200,16 @@ module.exports = React.createClass({ // forget what we wanted, so don't overwrite the saved state unless // this appears to be a user-initiated scroll. if (sn.scrollTop != this._lastSetScroll) { - // when scrolling, we don't care about disappearing typing notifs shrinking the timeline - // this might cause the scrollbar to resize in case the max-height was not correct - // but that's better than ending up with a lot of whitespace at the bottom of the timeline. - // we need to above check because when showing the typing notifs, an onScroll event is also triggered - if (!this.isAtBottom()) { - this.clearBlockShrinking(); - } - this._saveScrollState(); } else { debuglog("Ignoring scroll echo"); - // only ignore the echo once, otherwise we'll get confused when the // user scrolls away from, and back to, the autoscroll point. this._lastSetScroll = undefined; } + this._checkBlockShrinking(); + this.props.onScroll(ev); this.checkFillState(); @@ -228,8 +217,6 @@ module.exports = React.createClass({ onResize: function() { this.props.onResize(); - // clear min-height as the height might have changed - this.clearBlockShrinking(); this.checkScroll(); if (this._gemScroll) this._gemScroll.forceUpdate(); }, @@ -238,6 +225,7 @@ module.exports = React.createClass({ // where it ought to be, and set off pagination requests if necessary. checkScroll: function() { this._restoreSavedScrollState(); + this._checkBlockShrinking(); this.checkFillState(); }, @@ -379,8 +367,6 @@ module.exports = React.createClass({ } this._unfillDebouncer = setTimeout(() => { this._unfillDebouncer = null; - // if timeline shrinks, min-height should be cleared - this.clearBlockShrinking(); this.props.onUnfillRequest(backwards, markerScrollToken); }, UNFILL_REQUEST_DEBOUNCE_MS); } @@ -717,6 +703,21 @@ module.exports = React.createClass({ } }, + _checkBlockShrinking: function() { + const sn = this._getScrollNode(); + const scrollState = this.scrollState; + if (!scrollState.stuckAtBottom) { + const spaceBelowViewport = sn.scrollHeight - (sn.scrollTop + sn.clientHeight); + // only if we've scrolled up 200px from the bottom + // should we clear the min-height used by the typing notifications, + // otherwise we might still see it jump as the whitespace disappears + // when scrolling up from the bottom + if (spaceBelowViewport >= 200) { + this.clearBlockShrinking(); + } + } + }, + render: function() { const GeminiScrollbarWrapper = sdk.getComponent("elements.GeminiScrollbarWrapper"); // TODO: the classnames on the div and ol could do with being updated to From 38236428630bf0d93628939dcb3912f6a3ea5cd5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Feb 2019 13:16:16 +0100 Subject: [PATCH 05/34] some cleanup --- src/components/structures/ScrollPanel.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 154f7dfc12..ecfdfd5db2 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -590,7 +590,6 @@ module.exports = React.createClass({ let node = null; let nodeBottom; - // TODO: change this to not use getBoundingClientRect and save the domnode for (let i = messages.length-1; i >= 0; --i) { if (!messages[i].dataset.scrollTokens) { continue; @@ -623,7 +622,6 @@ module.exports = React.createClass({ _restoreSavedScrollState: function() { const scrollState = this.scrollState; - const scrollNode = this._getScrollNode(); if (scrollState.stuckAtBottom) { this._setScrollTop(Number.MAX_VALUE); From db7203ed7153cc44bad0ccca571bf722d815b3ce Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Feb 2019 14:48:33 +0100 Subject: [PATCH 06/34] make sure the min-height doesn't get cleared by checkScroll --- src/components/structures/MessagePanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 185af4cd6d..2e8197c1d7 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -635,9 +635,9 @@ module.exports = React.createClass({ _onTypingVisible: function() { const scrollPanel = this.refs.scrollPanel; if (scrollPanel && scrollPanel.getScrollState().stuckAtBottom) { - scrollPanel.blockShrinking(); // scroll down if at bottom scrollPanel.checkScroll(); + scrollPanel.blockShrinking(); } }, From 32f055bec26424911460b16ef5f14009b291e9c2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Feb 2019 14:48:56 +0100 Subject: [PATCH 07/34] clarify why we need this --- src/components/structures/MessagePanel.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 2e8197c1d7..291769fa03 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -648,6 +648,10 @@ module.exports = React.createClass({ const isAtBottom = scrollPanel.isAtBottom(); const whoIsTyping = this.refs.whoIsTyping; const isTypingVisible = whoIsTyping && whoIsTyping.isVisible(); + // when messages get added to the timeline, + // but somebody else is still typing, + // update the min-height, so once the last + // person stops typing, no jumping occurs if (isAtBottom && isTypingVisible) { scrollPanel.blockShrinking(); } From 8bb8ec141eea4230768c75b2eec3ea32f3c9e9fd Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Feb 2019 16:23:05 +0100 Subject: [PATCH 08/34] clear min-height on timeline resets and other occasions where we load it --- src/components/structures/MessagePanel.js | 7 +++++++ src/components/structures/TimelinePanel.js | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 291769fa03..9034123e52 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -658,6 +658,13 @@ module.exports = React.createClass({ } }, + clearTimelineHeight: function() { + const scrollPanel = this.refs.scrollPanel; + if (scrollPanel) { + scrollPanel.clearBlockShrinking(); + } + }, + onResize: function() { dis.dispatch({ action: 'timeline_resize' }, true); }, diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 8890c26d42..911499e314 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -935,6 +935,11 @@ var TimelinePanel = React.createClass({ {windowLimit: this.props.timelineCap}); const onLoaded = () => { + // clear the timeline min-height when + // (re)loading the timeline + if (this.refs.messagePanel) { + this.refs.messagePanel.clearTimelineHeight(); + } this._reloadEvents(); // If we switched away from the room while there were pending From ba5f16358f0233811c6e73086573f04c731930cb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Feb 2019 18:30:22 +0100 Subject: [PATCH 09/34] fall back to InteractionObserver for detecting timeline resizes this is not nearly as smooth as using ResizeObserver, as the callback rate is a lot lower, but seems to be quite a bit better than what we have right now, without the 7% cpu hog that the requestAnimationFrame polling fallback has. --- src/components/structures/ScrollPanel.js | 33 +++++++++++++++++++----- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index ecfdfd5db2..ecbb5ab868 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -78,6 +78,28 @@ if (DEBUG_SCROLL) { * scroll down further. If stickyBottom is disabled, we just save the scroll * offset as normal. */ + + +function createTimelineResizeDetector(scrollNode, itemlist, callback) { + if (typeof ResizeObserver !== "undefined") { + const ro = new ResizeObserver(callback); + ro.observe(itemlist); + return ro; + } else if (typeof IntersectionObserver !== "undefined") { + const threshold = []; + for (let i = 0; i < 1000; ++i) { + threshold.push(i / 1000); + } + threshold.push(1); + const io = new IntersectionObserver( + callback, + {root: scrollNode, threshold}, + ); + io.observe(itemlist); + return io; + } +} + module.exports = React.createClass({ displayName: 'ScrollPanel', @@ -161,12 +183,11 @@ module.exports = React.createClass({ componentDidMount: function() { this.checkScroll(); - if (typeof ResizeObserver !== "undefined") { - this._timelineSizeObserver = new ResizeObserver(() => { - this._restoreSavedScrollState(); - }); - this._timelineSizeObserver.observe(this.refs.itemlist); - } + this._timelineSizeObserver = createTimelineResizeDetector( + this._getScrollNode(), + this.refs.itemlist, + () => { this._restoreSavedScrollState(); }, + ); }, componentDidUpdate: function() { From 42030796c735ad913d20d95e4028f4a86a8c7f4c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 25 Feb 2019 18:48:42 +0100 Subject: [PATCH 10/34] remove test for #528 as we removed that fix --- .../components/structures/ScrollPanel-test.js | 53 ------------------- 1 file changed, 53 deletions(-) diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js index 0e091cdddf..6bdbcdb247 100644 --- a/test/components/structures/ScrollPanel-test.js +++ b/test/components/structures/ScrollPanel-test.js @@ -224,57 +224,4 @@ describe('ScrollPanel', function() { expect(scrollingDiv.scrollTop).toEqual(1950); }); }); - - it('should not get stuck in #528 workaround', function(done) { - let events = []; - Promise.resolve().then(() => { - // initialise with a bunch of events - for (let i = 0; i < 40; i++) { - events.push(i); - } - tester.setTileKeys(events); - expect(tester.fillCounts.b).toEqual(1); - expect(tester.fillCounts.f).toEqual(2); - expect(scrollingDiv.scrollHeight).toEqual(6050); // 40*150 + 50 - expect(scrollingDiv.scrollTop).toEqual(6050 - 600); - - // try to scroll up, to a non-integer offset. - tester.scrollPanel().scrollToToken("30", -101/3); - - expect(scrollingDiv.scrollTop).toEqual(4616); // 31*150 - 34 - - // wait for the scroll event to land - return tester.awaitScroll(); // fails - }).then(() => { - expect(tester.lastScrollEvent).toEqual(4616); - - // Now one more event; this will make it reset the scroll, but - // because the delta will be less than 1, will not trigger a - // scroll event, this leaving recentEventScroll defined. - console.log("Adding event 50"); - events.push(50); - tester.setTileKeys(events); - - // wait for the scrollpanel to stop trying to paginate - }).then(() => { - // Now, simulate hitting "scroll to bottom". - events = []; - for (let i = 100; i < 120; i++) { - events.push(i); - } - tester.setTileKeys(events); - tester.scrollPanel().scrollToBottom(); - - // wait for the scroll event to land - return tester.awaitScroll(); // fails - }).then(() => { - expect(scrollingDiv.scrollTop).toEqual(20*150 + 50 - 600); - - // simulate a user-initiated scroll on the div - scrollingDiv.scrollTop = 1200; - return tester.awaitScroll(); - }).then(() => { - expect(scrollingDiv.scrollTop).toEqual(1200); - }).done(done); - }); }); From c920dd2e8ad56281e9cd0fa3c7e9c04f0898ac0e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Feb 2019 16:26:24 +0100 Subject: [PATCH 11/34] check top of node instead of bottom, since coming in from last as we're approaching from the last node, if we're scrolled up, the first node we encounter would be below the bottom of the viewport change the logic to stop at the first node that has its top above the viewport bottom. When completely scrolled up, this was causing nodes way below the viewport to be selected as the reference for the pixelOffset, and when pagination came in, it would immediately try to apply the big negative pixelOffset, scrolling to a negative scrollTop, thus going to the top again, and triggering another pagination, entering in an infinite pagination loop until you scrolled down. --- src/components/structures/ScrollPanel.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index ecbb5ab868..10344a619f 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -609,20 +609,16 @@ module.exports = React.createClass({ const itemlist = this.refs.itemlist; const messages = itemlist.children; let node = null; - let nodeBottom; + // loop backwards, from bottom-most message (as that is the most common case) for (let i = messages.length-1; i >= 0; --i) { if (!messages[i].dataset.scrollTokens) { continue; } node = messages[i]; - - nodeBottom = node.offsetTop + node.clientHeight; - // If the bottom of the panel intersects the ClientRect of node, use this node - // as the scrollToken. - // If this is false for the entire for-loop, we default to the last node - // (which is why newScrollState is set on every iteration). - if (nodeBottom >= scrollBottom) { + // break at the first message (coming from the bottom) + // that has it's offsetTop above the bottom of the viewport. + if (node.offsetTop < scrollBottom) { // Use this node as the scrollToken break; } @@ -633,6 +629,7 @@ module.exports = React.createClass({ return; } + const nodeBottom = node.offsetTop + node.clientHeight; debuglog("ScrollPanel: saved scroll state", this.scrollState); this.scrollState = { stuckAtBottom: false, From fbe4d52b48871ca4a3e8f2ef529e5a9fb2fb60aa Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 26 Feb 2019 23:25:31 +0000 Subject: [PATCH 12/34] Fix Room Directory custom homeserver entry not showing propely Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/directory/NetworkDropdown.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/components/views/directory/NetworkDropdown.js b/src/components/views/directory/NetworkDropdown.js index c70cdeb0de..b2999e31cb 100644 --- a/src/components/views/directory/NetworkDropdown.js +++ b/src/components/views/directory/NetworkDropdown.js @@ -41,8 +41,8 @@ export default class NetworkDropdown extends React.Component { this.state = { expanded: false, selectedServer: server, - selectedInstance: null, - includeAllNetworks: false, + selectedInstanceId: null, + includeAllNetworks: true, }; } @@ -52,7 +52,8 @@ export default class NetworkDropdown extends React.Component { document.addEventListener('click', this.onDocumentClick, false); // fire this now so the defaults can be set up - this.props.onOptionChange(this.state.selectedServer, this.state.selectedInstance, this.state.includeAllNetworks); + const {selectedServer, selectedInstanceId, includeAllNetworks} = this.state; + this.props.onOptionChange(selectedServer, selectedInstanceId, includeAllNetworks); } componentWillUnmount() { @@ -97,17 +98,18 @@ export default class NetworkDropdown extends React.Component { expanded: false, selectedServer: server, selectedInstanceId: instance ? instance.instance_id : null, - includeAll: includeAll, + includeAllNetworks: includeAll, }); this.props.onOptionChange(server, instance ? instance.instance_id : null, includeAll); } onInputKeyUp(e) { - if (e.key == 'Enter') { + if (e.key === 'Enter') { this.setState({ expanded: false, selectedServer: e.target.value, selectedNetwork: null, + includeAllNetworks: true, }); this.props.onOptionChange(e.target.value, null); } @@ -227,7 +229,7 @@ export default class NetworkDropdown extends React.Component { } else { const instance = instanceForInstanceId(this.props.protocols, this.state.selectedInstanceId); current_value = this._makeMenuOption( - this.state.selectedServer, instance, this.state.includeAll, false, + this.state.selectedServer, instance, this.state.includeAllNetworks, false, ); } From 712522a16d960298487542c476c34fd1bc25db33 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Feb 2019 12:04:22 +0100 Subject: [PATCH 13/34] set chrome path for travis CI explicitly karma seems to be giving priority to a location where an old version is installed. --- scripts/travis/unit-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/travis/unit-tests.sh b/scripts/travis/unit-tests.sh index a8e0a63b31..a7e8425fa0 100755 --- a/scripts/travis/unit-tests.sh +++ b/scripts/travis/unit-tests.sh @@ -7,4 +7,4 @@ set -ev scripts/travis/build.sh -npm run test +CHROME_BIN='/usr/bin/google-chrome-stable' npm run test From ca1cbd9d67af06d9eb0fe9106f6cc8e2cc6cecde Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 28 Feb 2019 13:56:23 +0000 Subject: [PATCH 14/34] Nudge karma to 3.1.2 This includes a fix for detecting the version of Chrome used to run tests. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 84d5632023..124707294d 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,7 @@ "file-loader": "^3.0.1", "flow-parser": "^0.57.3", "jest-mock": "^23.2.0", - "karma": "^3.0.0", + "karma": "^3.1.2", "karma-chrome-launcher": "^0.2.3", "karma-cli": "^1.0.1", "karma-junit-reporter": "^0.4.2", From 69016e32a4396a0085f3e0f014ee7a61bb19ab04 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Feb 2019 15:39:10 +0100 Subject: [PATCH 15/34] fix margin on e2e icon in member panel --- res/css/views/rooms/_MemberInfo.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/css/views/rooms/_MemberInfo.scss b/res/css/views/rooms/_MemberInfo.scss index 8f89b83003..c3b3ca2f7d 100644 --- a/res/css/views/rooms/_MemberInfo.scss +++ b/res/css/views/rooms/_MemberInfo.scss @@ -27,7 +27,7 @@ limitations under the License. } .mx_MemberInfo_name > .mx_E2EIcon { - margin-left: 0; + margin-right: 0; } .mx_MemberInfo_cancel { From 0c06a702dc75ad542b63aeaf7f27fb485f230f87 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Feb 2019 16:05:55 +0100 Subject: [PATCH 16/34] pr feedback --- src/components/structures/ScrollPanel.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 10344a619f..2dbc711067 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -87,10 +87,9 @@ function createTimelineResizeDetector(scrollNode, itemlist, callback) { return ro; } else if (typeof IntersectionObserver !== "undefined") { const threshold = []; - for (let i = 0; i < 1000; ++i) { + for (let i = 0; i <= 1000; ++i) { threshold.push(i / 1000); } - threshold.push(1); const io = new IntersectionObserver( callback, {root: scrollNode, threshold}, From 0978ab3da0e0d2b2872f4970e21f173f9d7a2b45 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 28 Feb 2019 15:55:58 -0700 Subject: [PATCH 17/34] Support stacking dialogs to prevent unmounting Fixes https://github.com/vector-im/riot-web/issues/8371 --- res/css/_common.scss | 28 ++++++- src/Modal.js | 100 ++++++++++++++++++++---- src/components/structures/MatrixChat.js | 3 +- src/stores/RoomViewStore.js | 2 +- 4 files changed, 114 insertions(+), 19 deletions(-) diff --git a/res/css/_common.scss b/res/css/_common.scss index 9725340978..1e388c4531 100644 --- a/res/css/_common.scss +++ b/res/css/_common.scss @@ -228,6 +228,17 @@ textarea { color: $roomsublist-label-bg-color; } +/* Expected z-indexes for dialogs: + 4000 - Default wrapper index + 4009 - Static dialog background + 4010 - Static dialog itself + 4011 - Standard dialog background + 4012 - Standard dialog itself + + These are set up such that the static dialog always appears + underneath the standard dialogs. + */ + .mx_Dialog_wrapper { position: fixed; z-index: 4000; @@ -252,7 +263,7 @@ textarea { .mx_Dialog { background-color: $primary-bg-color; color: $light-fg-color; - z-index: 4010; + z-index: 4012; font-weight: 300; font-size: 15px; position: relative; @@ -264,6 +275,10 @@ textarea { overflow-y: auto; } +.mx_Dialog_staticWrapper .mx_Dialog { + z-index: 4010; +} + .mx_Dialog_background { position: fixed; top: 0; @@ -272,6 +287,17 @@ textarea { height: 100%; background-color: $dialog-backdrop-color; opacity: 0.8; + z-index: 4011; +} + +.mx_Dialog_background.mx_Dialog_staticBackground { + z-index: 4009; +} + +.mx_Dialog_wrapperWithStaticUnder .mx_Dialog_background { + // Roughly half of what it would normally be - we don't want to black out + // the app, just make it clear that the dialogs are stacked. + opacity: 0.4; } .mx_Dialog_lightbox .mx_Dialog_background { diff --git a/src/Modal.js b/src/Modal.js index 960e0e5c30..4d90e313ce 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -26,6 +26,7 @@ import dis from './dispatcher'; import { _t } from './languageHandler'; const DIALOG_CONTAINER_ID = "mx_Dialog_Container"; +const STATIC_DIALOG_CONTAINER_ID = "mx_Dialog_StaticContainer"; /** * Wrap an asynchronous loader function with a react component which shows a @@ -106,7 +107,12 @@ class ModalManager { // this modal. Remove all other modals from the stack when this modal // is closed. this._priorityModal = null; + // The modal to keep open underneath other modals if possible. Useful + // for cases like Settings where the modal should remain open while the + // user is prompted for more information/errors. + this._staticModal = null; // A list of the modals we have stacked up, with the most recent at [0] + // Neither the static nor priority modal will be in this list. this._modals = [ /* { elem: React component for this dialog @@ -130,6 +136,18 @@ class ModalManager { return container; } + getOrCreateStaticContainer() { + let container = document.getElementById(STATIC_DIALOG_CONTAINER_ID); + + if (!container) { + container = document.createElement("div"); + container.id = STATIC_DIALOG_CONTAINER_ID; + document.body.appendChild(container); + } + + return container; + } + createTrackedDialog(analyticsAction, analyticsInfo, ...rest) { Analytics.trackEvent('Modal', analyticsAction, analyticsInfo); return this.createDialog(...rest); @@ -166,8 +184,13 @@ class ModalManager { * of other modals that are currently in the stack. * Also, when closed, all modals will be removed * from the stack. + * @param {boolean} isStaticModal if true, this modal will be displayed under other + * modals in the stack. When closed, all modals will + * also be removed from the stack. This is not compatible + * with being a priority modal. Only one modal can be + * static at a time. */ - createDialogAsync(prom, props, className, isPriorityModal) { + createDialogAsync(prom, props, className, isPriorityModal, isStaticModal) { const self = this; const modal = {}; @@ -188,6 +211,13 @@ class ModalManager { self._modals = []; } + if (self._staticModal === modal) { + self._staticModal = null; + + // XXX: This is destructive + self._modals = []; + } + self._reRender(); }; @@ -207,6 +237,9 @@ class ModalManager { if (isPriorityModal) { // XXX: This is destructive this._priorityModal = modal; + } else if (isStaticModal) { + // This is intentionally destructive + this._staticModal = modal; } else { this._modals.unshift(modal); } @@ -216,12 +249,18 @@ class ModalManager { } closeAll() { - const modals = this._modals; + const modalsToClose = [...this._modals, this._priorityModal]; this._modals = []; + this._priorityModal = null; - for (let i = 0; i < modals.length; i++) { - const m = modals[i]; - if (m.onFinished) { + if (this._staticModal && modalsToClose.length === 0) { + modalsToClose.push(this._staticModal); + this._staticModal = null; + } + + for (let i = 0; i < modalsToClose.length; i++) { + const m = modalsToClose[i]; + if (m && m.onFinished) { m.onFinished(false); } } @@ -230,13 +269,14 @@ class ModalManager { } _reRender() { - if (this._modals.length == 0 && !this._priorityModal) { + if (this._modals.length === 0 && !this._priorityModal && !this._staticModal) { // If there is no modal to render, make all of Riot available // to screen reader users again dis.dispatch({ action: 'aria_unhide_main_app', }); ReactDOM.unmountComponentAtNode(this.getOrCreateContainer()); + ReactDOM.unmountComponentAtNode(this.getOrCreateStaticContainer()); return; } @@ -247,17 +287,45 @@ class ModalManager { action: 'aria_hide_main_app', }); - const modal = this._priorityModal ? this._priorityModal : this._modals[0]; - const dialog = ( -
-
- { modal.elem } -
-
-
- ); + if (this._staticModal) { + const classes = "mx_Dialog_wrapper mx_Dialog_staticWrapper " + + (this._staticModal.className ? this._staticModal.className : ''); - ReactDOM.render(dialog, this.getOrCreateContainer()); + const staticDialog = ( +
+
+ { this._staticModal.elem } +
+
+
+ ); + + ReactDOM.render(staticDialog, this.getOrCreateStaticContainer()); + } else { + // This is safe to call repeatedly if we happen to do that + ReactDOM.unmountComponentAtNode(this.getOrCreateStaticContainer()); + } + + const modal = this._priorityModal ? this._priorityModal : this._modals[0]; + if (modal) { + const classes = "mx_Dialog_wrapper " + + (this._staticModal ? "mx_Dialog_wrapperWithStaticUnder " : '') + + (modal.className ? modal.className : ''); + + const dialog = ( +
+
+ {modal.elem} +
+
+
+ ); + + ReactDOM.render(dialog, this.getOrCreateContainer()); + } else { + // This is safe to call repeatedly if we happen to do that + ReactDOM.unmountComponentAtNode(this.getOrCreateContainer()); + } } } diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 1bd0f02026..0a098c5f4f 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -584,7 +584,8 @@ export default React.createClass({ break; case 'view_user_settings': { const UserSettingsDialog = sdk.getComponent("dialogs.UserSettingsDialog"); - Modal.createTrackedDialog('User settings', '', UserSettingsDialog, {}, 'mx_SettingsDialog'); + Modal.createTrackedDialog('User settings', '', UserSettingsDialog, {}, 'mx_SettingsDialog', + /*isPriority=*/false, /*isStatic=*/true); // View the welcome or home page if we need something to look at this._viewSomethingBehindModal(); diff --git a/src/stores/RoomViewStore.js b/src/stores/RoomViewStore.js index 73eff6f6d4..d8556f661e 100644 --- a/src/stores/RoomViewStore.js +++ b/src/stores/RoomViewStore.js @@ -120,7 +120,7 @@ class RoomViewStore extends Store { const RoomSettingsDialog = sdk.getComponent("dialogs.RoomSettingsDialog"); Modal.createTrackedDialog('Room settings', '', RoomSettingsDialog, { roomId: payload.room_id || this._state.roomId, - }, 'mx_SettingsDialog'); + }, 'mx_SettingsDialog', /*isPriority=*/false, /*isStatic=*/true); break; } } From a41df7ab6858df2581d5ffbbf6564d028311a4a8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 7 Feb 2019 11:31:35 -0700 Subject: [PATCH 18/34] Convert PowerSelector to use mxField instead --- res/css/_components.scss | 1 + res/css/views/elements/_PowerSelector.scss | 25 +++++ .../views/elements/PowerSelector.js | 94 ++++++++----------- src/components/views/rooms/MemberInfo.js | 14 ++- .../tabs/room/RolesRoomSettingsTab.js | 69 ++++++-------- src/i18n/strings/en_EN.json | 37 ++++---- 6 files changed, 119 insertions(+), 121 deletions(-) create mode 100644 res/css/views/elements/_PowerSelector.scss diff --git a/res/css/_components.scss b/res/css/_components.scss index f3b07255ae..6f66a8c15e 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -85,6 +85,7 @@ @import "./views/elements/_InlineSpinner.scss"; @import "./views/elements/_ManageIntegsButton.scss"; @import "./views/elements/_MemberEventListSummary.scss"; +@import "./views/elements/_PowerSelector.scss"; @import "./views/elements/_ProgressBar.scss"; @import "./views/elements/_ReplyThread.scss"; @import "./views/elements/_ResizeHandle.scss"; diff --git a/res/css/views/elements/_PowerSelector.scss b/res/css/views/elements/_PowerSelector.scss new file mode 100644 index 0000000000..69f3a8eebb --- /dev/null +++ b/res/css/views/elements/_PowerSelector.scss @@ -0,0 +1,25 @@ +/* +Copyright 2019 New Vector Ltd. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +.mx_PowerSelector { + width: 100%; +} + +.mx_PowerSelector .mx_Field select, +.mx_PowerSelector .mx_Field input { + width: 100%; + box-sizing: border-box; +} diff --git a/src/components/views/elements/PowerSelector.js b/src/components/views/elements/PowerSelector.js index d1f102d9fe..83b8711bef 100644 --- a/src/components/views/elements/PowerSelector.js +++ b/src/components/views/elements/PowerSelector.js @@ -20,6 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import * as Roles from '../../../Roles'; import { _t } from '../../../languageHandler'; +import Field from "./Field"; module.exports = React.createClass({ displayName: 'PowerSelector', @@ -32,19 +33,15 @@ module.exports = React.createClass({ // Default user power level for the room usersDefault: PropTypes.number.isRequired, - // if true, the - ; - } - } - - let selectValue; - if (this.state.custom) { - selectValue = "SELECT_VALUE_CUSTOM"; - } else { - selectValue = this.state.levelRoleMap[this.props.value] ? - this.props.value : "SELECT_VALUE_CUSTOM"; - } - let select; - if (this.props.disabled) { - select = { this.state.levelRoleMap[selectValue] }; + picker = ( + + ); } else { // Each level must have a definition in this.state.levelRoleMap let options = this.state.options.map((level) => { @@ -145,20 +134,19 @@ module.exports = React.createClass({ return ; }); - select = - ; + picker = ( + + {options} + + ); } return ( - - { select } - { customPicker } - +
+ { picker } +
); }, }); diff --git a/src/components/views/rooms/MemberInfo.js b/src/components/views/rooms/MemberInfo.js index d7726c8fe8..3ada730ec8 100644 --- a/src/components/views/rooms/MemberInfo.js +++ b/src/components/views/rooms/MemberInfo.js @@ -947,14 +947,12 @@ module.exports = withMatrixClient(React.createClass({ const PowerSelector = sdk.getComponent('elements.PowerSelector'); roomMemberDetails =
- { _t("Level:") } - - +
{presenceLabel} diff --git a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.js b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.js index a6dac5a147..c4b1ae8ddc 100644 --- a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.js +++ b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.js @@ -24,14 +24,14 @@ import Modal from "../../../../../Modal"; const plEventsToLabels = { // These will be translated for us later. - "m.room.avatar": _td("To change the room's avatar, you must be a"), - "m.room.name": _td("To change the room's name, you must be a"), - "m.room.canonical_alias": _td("To change the room's main address, you must be a"), - "m.room.history_visibility": _td("To change the room's history visibility, you must be a"), - "m.room.power_levels": _td("To change the permissions in the room, you must be a"), - "m.room.topic": _td("To change the topic, you must be a"), + "m.room.avatar": _td("Change room avatar"), + "m.room.name": _td("Change room name"), + "m.room.canonical_alias": _td("Change main address for the room"), + "m.room.history_visibility": _td("Change history visibility"), + "m.room.power_levels": _td("Change permissions"), + "m.room.topic": _td("Change topic"), - "im.vector.modular.widgets": _td("To modify widgets in the room, you must be a"), + "im.vector.modular.widgets": _td("Modify widgets"), }; const plEventsToShow = { @@ -158,35 +158,35 @@ export default class RolesRoomSettingsTab extends React.Component { const powerLevelDescriptors = { "users_default": { - desc: _t('The default role for new room members is'), + desc: _t('Default role'), defaultValue: 0, }, "events_default": { - desc: _t('To send messages, you must be a'), + desc: _t('Send messages'), defaultValue: 0, }, "invite": { - desc: _t('To invite users into the room, you must be a'), + desc: _t('Invite users'), defaultValue: 50, }, "state_default": { - desc: _t('To configure the room, you must be a'), + desc: _t('Change settings'), defaultValue: 50, }, "kick": { - desc: _t('To kick users, you must be a'), + desc: _t('Kick users'), defaultValue: 50, }, "ban": { - desc: _t('To ban users, you must be a'), + desc: _t('Ban users'), defaultValue: 50, }, "redact": { - desc: _t('To remove other users\' messages, you must be a'), + desc: _t('Remove messages'), defaultValue: 50, }, "notifications.room": { - desc: _t('To notify everyone in the room, you must be a'), + desc: _t('Notify everyone'), defaultValue: 50, }, }; @@ -217,20 +217,15 @@ export default class RolesRoomSettingsTab extends React.Component { const mutedUsers = []; Object.keys(userLevels).forEach(function(user) { + const canChange = userLevels[user] < currentUserLevel && canChangeLevels; if (userLevels[user] > defaultUserLevel) { // privileged - privilegedUsers.push(
  • - { _t("%(user)s is a %(userRole)s", { - user: user, - userRole: , - }) } -
    • ); + privilegedUsers.push( + , + ); } else if (userLevels[user] < defaultUserLevel) { // muted - mutedUsers.push(
  • - { _t("%(user)s is a %(userRole)s", { - user: user, - userRole: , - }) } -
    • ); + mutedUsers.push( + , + ); } }); @@ -247,18 +242,14 @@ export default class RolesRoomSettingsTab extends React.Component { privilegedUsersSection =
    { _t('Privileged Users') }
    -
      - {privilegedUsers} -
    + {privilegedUsers}
    ; } if (mutedUsers.length) { mutedUsersSection =
    { _t('Muted Users') }
    -
      - {mutedUsers} -
    + {mutedUsers}
    ; } } @@ -300,11 +291,10 @@ export default class RolesRoomSettingsTab extends React.Component { const value = parseIntWithDefault(currentObj, descriptor.defaultValue); return
    - {descriptor.desc}  , you must be a", {}, - { 'eventType': { eventType } }, - ); + label = _t("Send %(eventType)s events", {eventType}); } return (
    - {label}  {_t("Permissions")} +

    {_t('Select the roles required to change various parts of the room')}

    {powerSelectors} {eventPowerSelectors}
    diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 696bd8152c..0d7cbd5f7f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -586,32 +586,32 @@ "Room Addresses": "Room Addresses", "Publish this room to the public in %(domain)s's room directory?": "Publish this room to the public in %(domain)s's room directory?", "URL Previews": "URL Previews", - "To change the room's avatar, you must be a": "To change the room's avatar, you must be a", - "To change the room's name, you must be a": "To change the room's name, you must be a", - "To change the room's main address, you must be a": "To change the room's main address, you must be a", - "To change the room's history visibility, you must be a": "To change the room's history visibility, you must be a", - "To change the permissions in the room, you must be a": "To change the permissions in the room, you must be a", - "To change the topic, you must be a": "To change the topic, you must be a", - "To modify widgets in the room, you must be a": "To modify widgets in the room, you must be a", + "Change room avatar": "Change room avatar", + "Change room name": "Change room name", + "Change main address for the room": "Change main address for the room", + "Change history visibility": "Change history visibility", + "Change permissions": "Change permissions", + "Change topic": "Change topic", + "Modify widgets": "Modify widgets", "Failed to unban": "Failed to unban", "Unban": "Unban", "Banned by %(displayName)s": "Banned by %(displayName)s", - "The default role for new room members is": "The default role for new room members is", - "To send messages, you must be a": "To send messages, you must be a", - "To invite users into the room, you must be a": "To invite users into the room, you must be a", - "To configure the room, you must be a": "To configure the room, you must be a", - "To kick users, you must be a": "To kick users, you must be a", - "To ban users, you must be a": "To ban users, you must be a", - "To remove other users' messages, you must be a": "To remove other users' messages, you must be a", - "To notify everyone in the room, you must be a": "To notify everyone in the room, you must be a", + "Default role": "Default role", + "Send messages": "Send messages", + "Invite users": "Invite users", + "Change settings": "Change settings", + "Kick users": "Kick users", + "Ban users": "Ban users", + "Remove messages": "Remove messages", + "Notify everyone": "Notify everyone", "No users have specific privileges in this room": "No users have specific privileges in this room", - "%(user)s is a %(userRole)s": "%(user)s is a %(userRole)s", "Privileged Users": "Privileged Users", "Muted Users": "Muted Users", "Banned users": "Banned users", - "To send events of type , you must be a": "To send events of type , you must be a", + "Send %(eventType)s events": "Send %(eventType)s events", "Roles & Permissions": "Roles & Permissions", "Permissions": "Permissions", + "Select the roles required to change various parts of the room": "Select the roles required to change various parts of the room", "Guests cannot join this room even if explicitly invited.": "Guests cannot join this room even if explicitly invited.", "Click here to fix": "Click here to fix", "To link to this room, please add an alias.": "To link to this room, please add an alias.", @@ -685,7 +685,6 @@ "Revoke Moderator": "Revoke Moderator", "Make Moderator": "Make Moderator", "Admin Tools": "Admin Tools", - "Level:": "Level:", "Close": "Close", "and %(count)s others...|other": "and %(count)s others...", "and %(count)s others...|one": "and one other...", @@ -998,7 +997,7 @@ "%(items)s and %(lastItem)s": "%(items)s and %(lastItem)s", "collapse": "collapse", "expand": "expand", - "Custom of %(powerLevel)s": "Custom of %(powerLevel)s", + "Power level": "Power level", "Custom level": "Custom level", "Unable to load event that was replied to, it either does not exist or you do not have permission to view it.": "Unable to load event that was replied to, it either does not exist or you do not have permission to view it.", "In reply to ": "In reply to ", From b84b11d3bdf147e426c00b710b141f3f5fd740ba Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 28 Feb 2019 19:13:28 -0700 Subject: [PATCH 19/34] Don't re-enable the save button after saving profiles Fixes https://github.com/vector-im/riot-web/issues/8569 --- src/components/views/room_settings/RoomProfileSettings.js | 1 - src/components/views/settings/ProfileSettings.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/components/views/room_settings/RoomProfileSettings.js b/src/components/views/room_settings/RoomProfileSettings.js index be3d9c5a37..8205ee3f6e 100644 --- a/src/components/views/room_settings/RoomProfileSettings.js +++ b/src/components/views/room_settings/RoomProfileSettings.js @@ -91,7 +91,6 @@ export default class RoomProfileSettings extends React.Component { newState.originalTopic = this.state.topic; } - newState.enableProfileSave = true; this.setState(newState); }; diff --git a/src/components/views/settings/ProfileSettings.js b/src/components/views/settings/ProfileSettings.js index 649128e4f3..01b94e42bb 100644 --- a/src/components/views/settings/ProfileSettings.js +++ b/src/components/views/settings/ProfileSettings.js @@ -72,7 +72,6 @@ export default class ProfileSettings extends React.Component { newState.avatarFile = null; } - newState.enableProfileSave = true; this.setState(newState); }; From 12d939b36fd0836cdec88b7b3d63878096621a41 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 28 Feb 2019 19:57:45 -0700 Subject: [PATCH 20/34] Support multiple email pushers and remove the legacy UserSettingsStore Fixes https://github.com/vector-im/riot-web/issues/5496 Fixes https://github.com/vector-im/riot-web/issues/8424 --- src/UserSettingsStore.js | 53 ------------------- .../views/settings/Notifications.js | 48 ++++++++++++----- 2 files changed, 36 insertions(+), 65 deletions(-) delete mode 100644 src/UserSettingsStore.js diff --git a/src/UserSettingsStore.js b/src/UserSettingsStore.js deleted file mode 100644 index c9ba1956ad..0000000000 --- a/src/UserSettingsStore.js +++ /dev/null @@ -1,53 +0,0 @@ -/* -Copyright 2015, 2016 OpenMarket Ltd -Copyright 2017, 2019 New Vector Ltd - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import MatrixClientPeg from './MatrixClientPeg'; - -// TODO: Decommission. -// Ref: https://github.com/vector-im/riot-web/issues/8424 -export default { - /* - * Returns the email pusher (pusher of type 'email') for a given - * email address. Email pushers all have the same app ID, so since - * pushers are unique over (app ID, pushkey), there will be at most - * one such pusher. - */ - getEmailPusher: function(pushers, address) { - if (pushers === undefined) { - return undefined; - } - for (let i = 0; i < pushers.length; ++i) { - if (pushers[i].kind === 'email' && pushers[i].pushkey === address) { - return pushers[i]; - } - } - return undefined; - }, - - addEmailPusher: function(address, data) { - return MatrixClientPeg.get().setPusher({ - kind: 'email', - app_id: 'm.email', - pushkey: address, - app_display_name: 'Email Notifications', - device_display_name: address, - lang: navigator.language, - data: data, - append: true, // We always append for email pushers since we don't want to stop other accounts notifying to the same email address - }); - }, -}; diff --git a/src/components/views/settings/Notifications.js b/src/components/views/settings/Notifications.js index 42c981fa03..b8f8279bb0 100644 --- a/src/components/views/settings/Notifications.js +++ b/src/components/views/settings/Notifications.js @@ -19,7 +19,6 @@ import Promise from 'bluebird'; import sdk from '../../../index'; import { _t } from '../../../languageHandler'; import MatrixClientPeg from '../../../MatrixClientPeg'; -import UserSettingsStore from '../../../UserSettingsStore'; import SettingsStore, {SettingLevel} from '../../../settings/SettingsStore'; import Modal from '../../../Modal'; import { @@ -132,14 +131,41 @@ module.exports = React.createClass({ }); }, + /* + * Returns the email pusher (pusher of type 'email') for a given + * email address. Email pushers all have the same app ID, so since + * pushers are unique over (app ID, pushkey), there will be at most + * one such pusher. + */ + getEmailPusher: function(pushers, address) { + if (pushers === undefined) { + return undefined; + } + for (let i = 0; i < pushers.length; ++i) { + if (pushers[i].kind === 'email' && pushers[i].pushkey === address) { + return pushers[i]; + } + } + return undefined; + }, + onEnableEmailNotificationsChange: function(address, checked) { let emailPusherPromise; if (checked) { const data = {}; data['brand'] = SdkConfig.get().brand || 'Riot'; - emailPusherPromise = UserSettingsStore.addEmailPusher(address, data); + emailPusherPromise = MatrixClientPeg.get().setPusher({ + kind: 'email', + app_id: 'm.email', + pushkey: address, + app_display_name: 'Email Notifications', + device_display_name: address, + lang: navigator.language, + data: data, + append: true, // We always append for email pushers since we don't want to stop other accounts notifying to the same email address + }); } else { - const emailPusher = UserSettingsStore.getEmailPusher(this.state.pushers, address); + const emailPusher = this.getEmailPusher(this.state.pushers, address); emailPusher.kind = null; emailPusherPromise = MatrixClientPeg.get().setPusher(emailPusher); } @@ -697,7 +723,7 @@ module.exports = React.createClass({ emailNotificationsRow: function(address, label) { return ; + label={label} key={`emailNotif_${label}`} />; }, render: function() { @@ -729,17 +755,15 @@ module.exports = React.createClass({ } const emailThreepids = this.state.threepids.filter((tp) => tp.medium === "email"); - let emailNotificationsRow; + let emailNotificationsRows; if (emailThreepids.length === 0) { - emailNotificationsRow =
    + emailNotificationsRows =
    { _t('Add an email address to configure email notifications') }
    ; } else { - // This only supports the first email address in your profile for now - emailNotificationsRow = this.emailNotificationsRow( - emailThreepids[0].address, - `${_t('Enable email notifications')} (${emailThreepids[0].address})`, - ); + emailNotificationsRows = emailThreepids.map((threePid) => this.emailNotificationsRow( + threePid.address, `${_t('Enable email notifications')} (${threePid.address})`, + )); } // Build external push rules @@ -823,7 +847,7 @@ module.exports = React.createClass({ onChange={this.onEnableAudioNotificationsChange} label={_t('Enable audible notifications in web client')} /> - { emailNotificationsRow } + { emailNotificationsRows }
    From 6b07195b63c0d1ea24209492f10ac9d8346f2955 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 1 Mar 2019 09:36:36 +0000 Subject: [PATCH 21/34] Add missing permalinkCreator prop Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/elements/ReplyThread.js | 4 ++-- src/components/views/rooms/MessageComposerInput.js | 2 +- src/components/views/rooms/ReplyPreview.js | 7 +++++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/ReplyThread.js b/src/components/views/elements/ReplyThread.js index e7dc020245..19b28d37a3 100644 --- a/src/components/views/elements/ReplyThread.js +++ b/src/components/views/elements/ReplyThread.js @@ -20,7 +20,7 @@ import PropTypes from 'prop-types'; import dis from '../../../dispatcher'; import {wantsDateSeparator} from '../../../DateUtils'; import {MatrixEvent, MatrixClient} from 'matrix-js-sdk'; -import {makeUserPermalink} from "../../../matrix-to"; +import {makeUserPermalink, RoomPermalinkCreator} from "../../../matrix-to"; import SettingsStore from "../../../settings/SettingsStore"; // This component does no cycle detection, simply because the only way to make such a cycle would be to @@ -32,7 +32,7 @@ export default class ReplyThread extends React.Component { parentEv: PropTypes.instanceOf(MatrixEvent), // called when the ReplyThread contents has changed, including EventTiles thereof onWidgetLoad: PropTypes.func.isRequired, - permalinkCreator: PropTypes.object.isRequired, + permalinkCreator: PropTypes.instanceOf(RoomPermalinkCreator).isRequired, }; static contextTypes = { diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 5ad09d1fa8..6b80902c8f 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -1592,7 +1592,7 @@ export default class MessageComposerInput extends React.Component { return (
    - + this.autocomplete = e} room={this.props.room} diff --git a/src/components/views/rooms/ReplyPreview.js b/src/components/views/rooms/ReplyPreview.js index 7656d21832..7599c5c308 100644 --- a/src/components/views/rooms/ReplyPreview.js +++ b/src/components/views/rooms/ReplyPreview.js @@ -20,6 +20,8 @@ import sdk from '../../../index'; import { _t } from '../../../languageHandler'; import RoomViewStore from '../../../stores/RoomViewStore'; import SettingsStore from "../../../settings/SettingsStore"; +import PropTypes from "prop-types"; +import {RoomPermalinkCreator} from "../../../matrix-to"; function cancelQuoting() { dis.dispatch({ @@ -29,6 +31,10 @@ function cancelQuoting() { } export default class ReplyPreview extends React.Component { + static propTypes = { + permalinkCreator: PropTypes.instanceOf(RoomPermalinkCreator).isRequired, + }; + constructor(props, context) { super(props, context); @@ -75,6 +81,7 @@ export default class ReplyPreview extends React.Component {
    ; From eb46e62a2e8d015c7b1ff4a95d46b6f0e0751c2a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 1 Mar 2019 09:39:39 +0000 Subject: [PATCH 22/34] delint NetworkDropdown Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .eslintignore.errorfiles | 1 - .../views/directory/NetworkDropdown.js | 26 ++++++++----------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/.eslintignore.errorfiles b/.eslintignore.errorfiles index d5eeebf1f2..c7d5804d66 100644 --- a/.eslintignore.errorfiles +++ b/.eslintignore.errorfiles @@ -18,7 +18,6 @@ src/components/views/create_room/RoomAlias.js src/components/views/dialogs/DeactivateAccountDialog.js src/components/views/dialogs/SetPasswordDialog.js src/components/views/dialogs/UnknownDeviceDialog.js -src/components/views/directory/NetworkDropdown.js src/components/views/elements/AddressSelector.js src/components/views/elements/DirectorySearchBox.js src/components/views/elements/ImageView.js diff --git a/src/components/views/directory/NetworkDropdown.js b/src/components/views/directory/NetworkDropdown.js index b2999e31cb..ebfff5ae8c 100644 --- a/src/components/views/directory/NetworkDropdown.js +++ b/src/components/views/directory/NetworkDropdown.js @@ -137,7 +137,7 @@ export default class NetworkDropdown extends React.Component { servers = servers.concat(this.props.config.roomDirectory.servers); } - if (servers.indexOf(MatrixClientPeg.getHomeServerName()) == -1) { + if (!servers.includes(MatrixClientPeg.getHomeServerName())) { servers.unshift(MatrixClientPeg.getHomeServerName()); } @@ -147,7 +147,7 @@ export default class NetworkDropdown extends React.Component { // we can only show the default room list. for (const server of servers) { options.push(this._makeMenuOption(server, null, true)); - if (server == MatrixClientPeg.getHomeServerName()) { + if (server === MatrixClientPeg.getHomeServerName()) { options.push(this._makeMenuOption(server, null, false)); if (this.props.protocols) { for (const proto of Object.keys(this.props.protocols)) { @@ -183,18 +183,15 @@ export default class NetworkDropdown extends React.Component { let icon; let name; - let span_class; let key; if (!instance && includeAll) { key = server; name = server; - span_class = 'mx_NetworkDropdown_menu_all'; } else if (!instance) { key = server + '_all'; name = 'Matrix'; icon = ; - span_class = 'mx_NetworkDropdown_menu_network'; } else { key = server + '_inst_' + instance.instance_id; const imgUrl = instance.icon ? @@ -202,41 +199,40 @@ export default class NetworkDropdown extends React.Component { DEFAULT_ICON_URL; icon = ; name = instance.desc; - span_class = 'mx_NetworkDropdown_menu_network'; } - const click_handler = handleClicks ? this.onMenuOptionClick.bind(this, server, instance, includeAll) : null; + const clickHandler = handleClicks ? this.onMenuOptionClick.bind(this, server, instance, includeAll) : null; - return
    + return
    {icon} {name}
    ; } render() { - let current_value; + let currentValue; let menu; if (this.state.expanded) { - const menu_options = this._getMenuOptions(); + const menuOptions = this._getMenuOptions(); menu =
    - {menu_options} + {menuOptions}
    ; - current_value = ; } else { const instance = instanceForInstanceId(this.props.protocols, this.state.selectedInstanceId); - current_value = this._makeMenuOption( + currentValue = this._makeMenuOption( this.state.selectedServer, instance, this.state.includeAllNetworks, false, ); } return
    - {current_value} - + {currentValue} + {menu}
    ; From 37593c117a9a18ceb91b7de2975e1b573d2a26f9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 1 Mar 2019 16:08:41 +0100 Subject: [PATCH 23/34] Revert "remove fix for old chrome bug" This reverts commit ecb074862ecfdf8a9c866d2c32438c2efc91111c. --- src/components/structures/ScrollPanel.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 2dbc711067..e9f0d12988 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -215,6 +215,24 @@ module.exports = React.createClass({ debuglog("Scroll event: offset now:", sn.scrollTop, "_lastSetScroll:", this._lastSetScroll); + // Sometimes we see attempts to write to scrollTop essentially being + // ignored. (Or rather, it is successfully written, but on the next + // scroll event, it's been reset again). + // + // This was observed on Chrome 47, when scrolling using the trackpad in OS + // X Yosemite. Can't reproduce on El Capitan. Our theory is that this is + // due to Chrome not being able to cope with the scroll offset being reset + // while a two-finger drag is in progress. + // + // By way of a workaround, we detect this situation and just keep + // resetting scrollTop until we see the scroll node have the right + // value. + if (this._lastSetScroll !== undefined && sn.scrollTop < this._lastSetScroll-200) { + console.log("Working around vector-im/vector-web#528"); + this._restoreSavedScrollState(); + return; + } + // If there weren't enough children to fill the viewport, the scroll we // got might be different to the scroll we wanted; we don't want to // forget what we wanted, so don't overwrite the saved state unless From 65807c7a6655bd322332d69785d134dcab18348d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 1 Mar 2019 16:08:58 +0100 Subject: [PATCH 24/34] Revert "remove test for #528 as we removed that fix" This reverts commit 42030796c735ad913d20d95e4028f4a86a8c7f4c. --- .../components/structures/ScrollPanel-test.js | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js index 6bdbcdb247..0e091cdddf 100644 --- a/test/components/structures/ScrollPanel-test.js +++ b/test/components/structures/ScrollPanel-test.js @@ -224,4 +224,57 @@ describe('ScrollPanel', function() { expect(scrollingDiv.scrollTop).toEqual(1950); }); }); + + it('should not get stuck in #528 workaround', function(done) { + let events = []; + Promise.resolve().then(() => { + // initialise with a bunch of events + for (let i = 0; i < 40; i++) { + events.push(i); + } + tester.setTileKeys(events); + expect(tester.fillCounts.b).toEqual(1); + expect(tester.fillCounts.f).toEqual(2); + expect(scrollingDiv.scrollHeight).toEqual(6050); // 40*150 + 50 + expect(scrollingDiv.scrollTop).toEqual(6050 - 600); + + // try to scroll up, to a non-integer offset. + tester.scrollPanel().scrollToToken("30", -101/3); + + expect(scrollingDiv.scrollTop).toEqual(4616); // 31*150 - 34 + + // wait for the scroll event to land + return tester.awaitScroll(); // fails + }).then(() => { + expect(tester.lastScrollEvent).toEqual(4616); + + // Now one more event; this will make it reset the scroll, but + // because the delta will be less than 1, will not trigger a + // scroll event, this leaving recentEventScroll defined. + console.log("Adding event 50"); + events.push(50); + tester.setTileKeys(events); + + // wait for the scrollpanel to stop trying to paginate + }).then(() => { + // Now, simulate hitting "scroll to bottom". + events = []; + for (let i = 100; i < 120; i++) { + events.push(i); + } + tester.setTileKeys(events); + tester.scrollPanel().scrollToBottom(); + + // wait for the scroll event to land + return tester.awaitScroll(); // fails + }).then(() => { + expect(scrollingDiv.scrollTop).toEqual(20*150 + 50 - 600); + + // simulate a user-initiated scroll on the div + scrollingDiv.scrollTop = 1200; + return tester.awaitScroll(); + }).then(() => { + expect(scrollingDiv.scrollTop).toEqual(1200); + }).done(done); + }); }); From e1f26059a767a01d522c942433eecfb862049458 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 09:58:54 -0700 Subject: [PATCH 25/34] Convert objects and such to usable strings in rageshake Fixes https://github.com/vector-im/riot-web/issues/7844 --- src/rageshake/rageshake.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/rageshake/rageshake.js b/src/rageshake/rageshake.js index 29dbe4f41d..6366392c04 100644 --- a/src/rageshake/rageshake.js +++ b/src/rageshake/rageshake.js @@ -71,6 +71,18 @@ class ConsoleLogger { log(level, ...args) { // We don't know what locale the user may be running so use ISO strings const ts = new Date().toISOString(); + + // Convert objects and errors to helpful things + args = args.map((arg) => { + if (arg instanceof Error) { + return arg.message + (arg.stack ? `\n${arg.stack}` : ''); + } else if (typeof(arg) === 'object') { + return JSON.stringify(arg); + } else { + return arg; + } + }); + // Some browsers support string formatting which we're not doing here // so the lines are a little more ugly but easy to implement / quick to // run. From 4c4b2eedafe0029d9cccdb45df9de7e18dca7b3f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 13:36:24 -0700 Subject: [PATCH 26/34] Check if users are already in the room before inviting them Fixes https://github.com/vector-im/riot-web/issues/8965 This also addresses another issue where inviting a banned user shows up as "Unknown server error". --- src/i18n/strings/en_EN.json | 2 ++ src/utils/MultiInviter.js | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 0d7cbd5f7f..19259c6341 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -238,8 +238,10 @@ "Authentication check failed: incorrect password?": "Authentication check failed: incorrect password?", "Unrecognised address": "Unrecognised address", "You do not have permission to invite people to this room.": "You do not have permission to invite people to this room.", + "User %(userId)s is already in the room": "User %(userId)s is already in the room", "User %(user_id)s does not exist": "User %(user_id)s does not exist", "User %(user_id)s may or may not exist": "User %(user_id)s may or may not exist", + "The user must be unbanned before they can be invited.": "The user must be unbanned before they can be invited.", "Unknown server error": "Unknown server error", "Use a few words, avoid common phrases": "Use a few words, avoid common phrases", "No need for symbols, digits, or uppercase letters": "No need for symbols, digits, or uppercase letters", diff --git a/src/utils/MultiInviter.js b/src/utils/MultiInviter.js index 4278ee1c90..2e166b1925 100644 --- a/src/utils/MultiInviter.js +++ b/src/utils/MultiInviter.js @@ -101,6 +101,14 @@ export default class MultiInviter { if (addrType === 'email') { return MatrixClientPeg.get().inviteByEmail(roomId, addr); } else if (addrType === 'mx-user-id') { + const room = MatrixClientPeg.get().getRoom(roomId); + if (!room) throw new Error("Room not found"); + + const member = room.getMember(addr); + if (member && ['join', 'invite'].includes(member.membership)) { + throw {errcode: "RIOT.ALREADY_IN_ROOM", error: "Member already invited"}; + } + if (!ignoreProfile && SettingsStore.getValue("promptBeforeInviteUnknownUsers", this.roomId)) { try { const profile = await MatrixClientPeg.get().getProfileInfo(addr); @@ -152,6 +160,8 @@ export default class MultiInviter { if (err.errcode === 'M_FORBIDDEN') { fatal = true; errorText = _t('You do not have permission to invite people to this room.'); + } else if (err.errcode === "RIOT.ALREADY_IN_ROOM") { + errorText = _t("User %(userId)s is already in the room", {userId: address}); } else if (err.errcode === 'M_LIMIT_EXCEEDED') { // we're being throttled so wait a bit & try again setTimeout(() => { @@ -166,6 +176,8 @@ export default class MultiInviter { // Invite without the profile check console.warn(`User ${address} does not have a profile - inviting anyways automatically`); this._doInvite(address, true).then(resolve, reject); + } else if (err.errcode === "M_BAD_STATE") { + errorText = _t("The user must be unbanned before they can be invited."); } else { errorText = _t('Unknown server error'); } From 3c2403ca5492e48ac5865ffc1831ca2f5775065c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 15:29:24 -0700 Subject: [PATCH 27/34] Don't duplicate direct chats from other tags Fixes https://github.com/vector-im/riot-web/issues/8971 --- src/stores/RoomListStore.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index e9ac33b506..8355e29f6c 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -319,7 +319,9 @@ class RoomListStore extends Store { const dmRoomMap = DMRoomMap.shared(); if (myMembership === 'invite') { tags.push("im.vector.fake.invite"); - } else if (dmRoomMap.getUserIdForRoomId(room.roomId)) { + } else if (dmRoomMap.getUserIdForRoomId(room.roomId) && tags.length === 0) { + // We intentionally don't duplicate rooms in other tags into the people list + // as a feature. tags.push("im.vector.fake.direct"); } else if (tags.length === 0) { tags.push("im.vector.fake.recent"); From 43d099836ba7b44c93a190a8b9773b75e6271217 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 15:48:10 -0700 Subject: [PATCH 28/34] Always insert rooms into lists when they get lost Room upgrades, direct chats, etc all end up being lost in these scenarios. Instead of losing them to the list, try and put them into a relevant spot of the list. Fixes https://github.com/vector-im/riot-web/issues/9020 --- src/stores/RoomListStore.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index e9ac33b506..fc140f31e6 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -477,18 +477,12 @@ class RoomListStore extends Store { room, category, this._state.lists[key], listsClone[key], lastTimestamp); if (!pushedEntry) { - // Special case invites: they don't really have timelines and can easily get lost when - // the user has multiple pending invites. Pushing them is the least worst option. - if (listsClone[key].length === 0 || key === "im.vector.fake.invite") { - listsClone[key].push({room, category}); - insertedIntoTags.push(key); - } else { - // In theory, this should never happen - console.warn(`!! Room ${room.roomId} lost: No position available`); - } - } else { - insertedIntoTags.push(key); + // There's some circumstances where the room doesn't fit anywhere, so just + // push the room in. We push it in at the start of the list because the room + // is probably important. + listsClone[key].splice(0, 0, {room, category}); } + insertedIntoTags.push(key); } } From 8a6ae6b48edc81033f40f556cff54a20f8f22f4c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 15:59:08 -0700 Subject: [PATCH 29/34] Use a mask for the continuation icon Fixes https://github.com/vector-im/riot-web/issues/7990 --- res/css/views/messages/_CreateEvent.scss | 7 ++++++- src/components/views/messages/RoomCreate.js | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/res/css/views/messages/_CreateEvent.scss b/res/css/views/messages/_CreateEvent.scss index c095fc26af..adf16d6c4a 100644 --- a/res/css/views/messages/_CreateEvent.scss +++ b/res/css/views/messages/_CreateEvent.scss @@ -24,9 +24,14 @@ limitations under the License. .mx_CreateEvent_image { float: left; - padding-right: 20px; + margin-right: 20px; width: 72px; height: 34px; + + background-color: $primary-fg-color; + mask: url('$(res)/img/room-continuation.svg'); + mask-repeat: no-repeat; + mask-position: center; } .mx_CreateEvent_header { diff --git a/src/components/views/messages/RoomCreate.js b/src/components/views/messages/RoomCreate.js index 978448ba4f..27395fce5c 100644 --- a/src/components/views/messages/RoomCreate.js +++ b/src/components/views/messages/RoomCreate.js @@ -53,7 +53,7 @@ module.exports = React.createClass({ permalinkCreator.load(); const predecessorPermalink = permalinkCreator.forEvent(predecessor['event_id']); return
    - +
    {_t("This room is a continuation of another conversation.")}
    From 49f506cef49e14e6c7b2adec71f0076388e25c56 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 17:18:16 -0700 Subject: [PATCH 30/34] More clearly fix issues with room insertion to lists Instead of having a catch-all insert, try and fix the common cases with a bit more care. --- src/stores/RoomListStore.js | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index fc140f31e6..78e0659e36 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -429,6 +429,13 @@ class RoomListStore extends Store { newList.push(entry); } + if (!pushedEntry && desiredCategoryBoundaryIndex >= 0) { + console.warn(`!! Room ${room.roomId} nearly lost: Ran off the end of the list`); + console.warn(`!! Inserting at position ${desiredCategoryBoundaryIndex} with category ${category}`); + newList.splice(desiredCategoryBoundaryIndex, 0, {room, category}); + pushedEntry = true; + } + return pushedEntry; } @@ -477,16 +484,27 @@ class RoomListStore extends Store { room, category, this._state.lists[key], listsClone[key], lastTimestamp); if (!pushedEntry) { - // There's some circumstances where the room doesn't fit anywhere, so just - // push the room in. We push it in at the start of the list because the room - // is probably important. + // This should rarely happen: _slotRoomIntoList has several checks which attempt + // to make sure that a room is not lost in the list. If we do lose the room though, + // we shouldn't throw it on the floor and forget about it. Instead, we should insert + // it somewhere. We'll insert it at the top for a couple reasons: 1) it is probably + // an important room for the user and 2) if this does happen, we'd want a bug report. + console.warn(`!! Room ${room.roomId} nearly lost: Failed to find a position`); + console.warn(`!! Inserting at position 0 in the list and flagging as inserted`); + console.warn("!! Additional info: ", { + category, + key, + upToIndex: listsClone[key].length, + expectedCount: this._state.lists[key].length, + }); listsClone[key].splice(0, 0, {room, category}); } insertedIntoTags.push(key); } } - // Double check that we inserted the room in the right places + // Double check that we inserted the room in the right places. + // There should never be a discrepancy. for (const targetTag of targetTags) { let count = 0; for (const insertedTag of insertedIntoTags) { From 454251d17c5b116c1df24037d118b045dabdee01 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 1 Mar 2019 17:21:37 -0700 Subject: [PATCH 31/34] Use a div instead of an image There's no source, so just make it a block element --- src/components/views/messages/RoomCreate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/messages/RoomCreate.js b/src/components/views/messages/RoomCreate.js index 27395fce5c..f9dc3df2dc 100644 --- a/src/components/views/messages/RoomCreate.js +++ b/src/components/views/messages/RoomCreate.js @@ -53,7 +53,7 @@ module.exports = React.createClass({ permalinkCreator.load(); const predecessorPermalink = permalinkCreator.forEvent(predecessor['event_id']); return
    - +
    {_t("This room is a continuation of another conversation.")}
    From 71630af20115c0234f305080fa934a4288e8d448 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 2 Mar 2019 10:59:25 -0700 Subject: [PATCH 32/34] Patch users not existing when opening settings Fixes https://github.com/vector-im/riot-web/issues/9022 --- src/components/views/settings/ProfileSettings.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/components/views/settings/ProfileSettings.js b/src/components/views/settings/ProfileSettings.js index 01b94e42bb..1bb54d9361 100644 --- a/src/components/views/settings/ProfileSettings.js +++ b/src/components/views/settings/ProfileSettings.js @@ -20,13 +20,22 @@ import MatrixClientPeg from "../../../MatrixClientPeg"; import Field from "../elements/Field"; import AccessibleButton from "../elements/AccessibleButton"; import classNames from 'classnames'; +import {User} from "matrix-js-sdk"; export default class ProfileSettings extends React.Component { constructor() { super(); const client = MatrixClientPeg.get(); - const user = client.getUser(client.getUserId()); + let user = client.getUser(client.getUserId()); + if (!user) { + // XXX: We shouldn't have to do this. + // There seems to be a condition where the User object won't exist until a room + // exists on the account. To work around this, we'll just create a temporary User + // and use that. + console.warn("User object not found - creating one for ProfileSettings"); + user = new User(client.getUserId()); + } let avatarUrl = user.avatarUrl; if (avatarUrl) avatarUrl = client.mxcUrlToHttp(avatarUrl, 96, 96, 'crop', false); this.state = { From c7b019830d8b8cdc32119b624a2139c0eb75de44 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 4 Mar 2019 14:35:41 +0000 Subject: [PATCH 33/34] Fix media device selectors not updating Missed a setState Fixes https://github.com/vector-im/riot-web/issues/9036 --- .../views/settings/tabs/user/VoiceUserSettingsTab.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/components/views/settings/tabs/user/VoiceUserSettingsTab.js b/src/components/views/settings/tabs/user/VoiceUserSettingsTab.js index 31791708e0..84d70a48d4 100644 --- a/src/components/views/settings/tabs/user/VoiceUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/VoiceUserSettingsTab.js @@ -76,14 +76,23 @@ export default class VoiceUserSettingsTab extends React.Component { _setAudioOutput = (e) => { CallMediaHandler.setAudioOutput(e.target.value); + this.setState({ + activeAudioOutput: e.target.value, + }); }; _setAudioInput = (e) => { CallMediaHandler.setAudioInput(e.target.value); + this.setState({ + activeAudioInput: e.target.value, + }); }; _setVideoInput = (e) => { CallMediaHandler.setVideoInput(e.target.value); + this.setState({ + activeVideoInput: e.target.value, + }); }; _changeWebRtcMethod = (p2p) => { From 363964e423b0ef240324b9bcda1273e8ea925ad3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 4 Mar 2019 13:08:54 -0700 Subject: [PATCH 34/34] Don't loop forever if you demote yourself via Enter on a PowerSelector The included comment explains what happens and why this is bad. Fixes https://github.com/vector-im/riot-web/issues/9039 --- src/components/views/elements/PowerSelector.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/PowerSelector.js b/src/components/views/elements/PowerSelector.js index 83b8711bef..89d46de117 100644 --- a/src/components/views/elements/PowerSelector.js +++ b/src/components/views/elements/PowerSelector.js @@ -103,12 +103,23 @@ module.exports = React.createClass({ }, onCustomBlur: function(event) { + event.preventDefault(); + event.stopPropagation(); + this.props.onChange(parseInt(this.state.customValue), this.props.powerLevelKey); }, - onCustomKeyDown: function(event) { + onCustomKeyPress: function(event) { if (event.key === "Enter") { - this.props.onChange(parseInt(this.state.customValue), this.props.powerLevelKey); + event.preventDefault(); + event.stopPropagation(); + + // Do not call the onChange handler directly here - it can cause an infinite loop. + // Long story short, a user hits Enter to submit the value which onChange handles as + // raising a dialog which causes a blur which causes a dialog which causes a blur and + // so on. By not causing the onChange to be called here, we avoid the loop because we + // handle the onBlur safely. + event.target.blur(); } }, @@ -118,7 +129,7 @@ module.exports = React.createClass({ picker = ( ); } else {