From 0590ce7faf9680d9d720a43de786545e7da5e7e6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 23 Apr 2017 06:06:23 +0100 Subject: [PATCH 01/11] Conform damn you (mostly) Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Notifier.js | 53 ++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/Notifier.js b/src/Notifier.js index 92770877b7..617135a2c8 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -15,11 +15,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -var MatrixClientPeg = require("./MatrixClientPeg"); -var PlatformPeg = require("./PlatformPeg"); -var TextForEvent = require('./TextForEvent'); -var Avatar = require('./Avatar'); -var dis = require("./dispatcher"); +import MatrixClientPeg from './MatrixClientPeg'; +import PlatformPeg from './PlatformPeg'; +import TextForEvent from './TextForEvent'; +import Avatar from './Avatar'; +import dis from './dispatcher'; /* * Dispatches: @@ -29,7 +29,7 @@ var dis = require("./dispatcher"); * } */ -var Notifier = { +const Notifier = { notifsByRoom: {}, notificationMessageForEvent: function(ev) { @@ -48,16 +48,16 @@ var Notifier = { return; } - var msg = this.notificationMessageForEvent(ev); + let msg = this.notificationMessageForEvent(ev); if (!msg) return; - var title; - if (!ev.sender || room.name == ev.sender.name) { + let title; + if (!ev.sender || room.name === ev.sender.name) { title = room.name; // notificationMessageForEvent includes sender, // but we already have the sender here if (ev.getContent().body) msg = ev.getContent().body; - } else if (ev.getType() == 'm.room.member') { + } else if (ev.getType() === 'm.room.member') { // context is all in the message here, we don't need // to display sender info title = room.name; @@ -68,7 +68,7 @@ var Notifier = { if (ev.getContent().body) msg = ev.getContent().body; } - var avatarUrl = ev.sender ? Avatar.avatarUrlForMember( + const avatarUrl = ev.sender ? Avatar.avatarUrlForMember( ev.sender, 40, 40, 'crop' ) : null; @@ -83,7 +83,7 @@ var Notifier = { }, _playAudioNotification: function(ev, room) { - var e = document.getElementById("messageAudio"); + const e = document.getElementById("messageAudio"); if (e) { e.load(); e.play(); @@ -95,7 +95,7 @@ var Notifier = { this.boundOnSyncStateChange = this.onSyncStateChange.bind(this); this.boundOnRoomReceipt = this.onRoomReceipt.bind(this); MatrixClientPeg.get().on('Room.timeline', this.boundOnRoomTimeline); - MatrixClientPeg.get().on("Room.receipt", this.boundOnRoomReceipt); + MatrixClientPeg.get().on('Room.receipt', this.boundOnRoomReceipt); MatrixClientPeg.get().on("sync", this.boundOnSyncStateChange); this.toolbarHidden = false; this.isSyncing = false; @@ -104,7 +104,7 @@ var Notifier = { stop: function() { if (MatrixClientPeg.get() && this.boundOnRoomTimeline) { MatrixClientPeg.get().removeListener('Room.timeline', this.boundOnRoomTimeline); - MatrixClientPeg.get().removeListener("Room.receipt", this.boundOnRoomReceipt); + MatrixClientPeg.get().removeListener('Room.receipt', this.boundOnRoomReceipt); MatrixClientPeg.get().removeListener('sync', this.boundOnSyncStateChange); } this.isSyncing = false; @@ -121,7 +121,7 @@ var Notifier = { // make sure that we persist the current setting audio_enabled setting // before changing anything if (global.localStorage) { - if(global.localStorage.getItem('audio_notifications_enabled') == null) { + if (global.localStorage.getItem('audio_notifications_enabled') === null) { this.setAudioEnabled(this.isEnabled()); } } @@ -141,7 +141,7 @@ var Notifier = { if (callback) callback(); dis.dispatch({ action: "notifier_enabled", - value: true + value: true, }); }); // clear the notifications_hidden flag, so that if notifications are @@ -152,7 +152,7 @@ var Notifier = { global.localStorage.setItem('notifications_enabled', 'false'); dis.dispatch({ action: "notifier_enabled", - value: false + value: false, }); } }, @@ -165,7 +165,7 @@ var Notifier = { if (!global.localStorage) return true; - var enabled = global.localStorage.getItem('notifications_enabled'); + const enabled = global.localStorage.getItem('notifications_enabled'); if (enabled === null) return true; return enabled === 'true'; }, @@ -173,12 +173,12 @@ var Notifier = { setAudioEnabled: function(enable) { if (!global.localStorage) return; global.localStorage.setItem('audio_notifications_enabled', - enable ? 'true' : 'false'); + enable ? 'true' : 'false'); }, isAudioEnabled: function(enable) { if (!global.localStorage) return true; - var enabled = global.localStorage.getItem( + const enabled = global.localStorage.getItem( 'audio_notifications_enabled'); // default to true if the popups are enabled if (enabled === null) return this.isEnabled(); @@ -192,7 +192,7 @@ var Notifier = { // this is nothing to do with notifier_enabled dis.dispatch({ action: "notifier_enabled", - value: this.isEnabled() + value: this.isEnabled(), }); // update the info to localStorage for persistent settings @@ -215,8 +215,7 @@ var Notifier = { onSyncStateChange: function(state) { if (state === "SYNCING") { this.isSyncing = true; - } - else if (state === "STOPPED" || state === "ERROR") { + } else if (state === "STOPPED" || state === "ERROR") { this.isSyncing = false; } }, @@ -225,10 +224,10 @@ var Notifier = { if (toStartOfTimeline) return; if (!room) return; if (!this.isSyncing) return; // don't alert for any messages initially - if (ev.sender && ev.sender.userId == MatrixClientPeg.get().credentials.userId) return; + if (ev.sender && ev.sender.userId === MatrixClientPeg.get().credentials.userId) return; if (data.timeline.getTimelineSet() !== room.getUnfilteredTimelineSet()) return; - var actions = MatrixClientPeg.get().getPushActionsForEvent(ev); + const actions = MatrixClientPeg.get().getPushActionsForEvent(ev); if (actions && actions.notify) { if (this.isEnabled()) { this._displayPopupNotification(ev, room); @@ -240,7 +239,7 @@ var Notifier = { }, onRoomReceipt: function(ev, room) { - if (room.getUnreadNotificationCount() == 0) { + if (room.getUnreadNotificationCount() === 0) { // ideally we would clear each notification when it was read, // but we have no way, given a read receipt, to know whether // the receipt comes before or after an event, so we can't @@ -255,7 +254,7 @@ var Notifier = { } delete this.notifsByRoom[room.roomId]; } - } + }, }; if (!global.mxNotifier) { From 5e8b43f3edc70cd8d73a920c9a231c0f681c43fb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 23 Apr 2017 06:16:25 +0100 Subject: [PATCH 02/11] if we're not granted, show an ErrorDialog with some text which needs changing Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Notifier.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Notifier.js b/src/Notifier.js index 617135a2c8..fed2760732 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -20,6 +20,8 @@ import PlatformPeg from './PlatformPeg'; import TextForEvent from './TextForEvent'; import Avatar from './Avatar'; import dis from './dispatcher'; +import sdk from './index'; +import Modal from './Modal'; /* * Dispatches: @@ -131,6 +133,14 @@ const Notifier = { plaf.requestNotificationPermission().done((result) => { if (result !== 'granted') { // The permission request was dismissed or denied + const description = result === 'denied' + ? 'Your browser is not permitting this app to send you notifications.' + : 'It seems you didn\'t accept notifications when your browser asked'; + const ErrorDialog = sdk.getComponent('dialogs.ErrorDialog'); + Modal.createDialog(ErrorDialog, { + title: 'Unable to enable Notifications', + description, + }); return; } From c9c72036f38e500a88f5b9e0b7b57808a583f0ab Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@googlemail.com> Date: Fri, 28 Apr 2017 01:00:10 +0100 Subject: [PATCH 03/11] Change wording Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Notifier.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Notifier.js b/src/Notifier.js index fed2760732..f68b7e562c 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -134,8 +134,10 @@ const Notifier = { if (result !== 'granted') { // The permission request was dismissed or denied const description = result === 'denied' - ? 'Your browser is not permitting this app to send you notifications.' - : 'It seems you didn\'t accept notifications when your browser asked'; + ? 'Riot does not have permission to send you notifications' + + ' - please check your browser settings' + : 'Riot was not given permission to send notifications' + + '- please try again'; const ErrorDialog = sdk.getComponent('dialogs.ErrorDialog'); Modal.createDialog(ErrorDialog, { title: 'Unable to enable Notifications', From 47827e0b81c8db6977d2a628451daf9db348c5b5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@googlemail.com> Date: Fri, 28 Apr 2017 01:00:50 +0100 Subject: [PATCH 04/11] un-eat the space Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Notifier.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Notifier.js b/src/Notifier.js index f68b7e562c..6473ab4d9c 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -137,7 +137,7 @@ const Notifier = { ? 'Riot does not have permission to send you notifications' + ' - please check your browser settings' : 'Riot was not given permission to send notifications' - + '- please try again'; + + ' - please try again'; const ErrorDialog = sdk.getComponent('dialogs.ErrorDialog'); Modal.createDialog(ErrorDialog, { title: 'Unable to enable Notifications', From c8fb18dc932bdfe6e64bce328b515a52d6c32607 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@googlemail.com> Date: Sun, 30 Apr 2017 13:00:47 +0100 Subject: [PATCH 05/11] Pin filesize ver to fix break upstream https://travis-ci.org/vector-im/riot-web/builds/227340622 https://github.com/avoidwork/filesize.js/issues/87 3.5.7 and 3.5.8 ver released <24h ago and broke stuff for us --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5c96a74f5b..95a82bbb73 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "draft-js-export-markdown": "^0.2.0", "emojione": "2.2.3", "file-saver": "^1.3.3", - "filesize": "^3.1.2", + "filesize": "3.5.6", "flux": "^2.0.3", "fuse.js": "^2.2.0", "glob": "^5.0.14", From 3f25928380b36834e0cbe249938581ede9dc3c6e Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 2 May 2017 16:34:39 +0100 Subject: [PATCH 06/11] Fix jumping to an unread event when in MELS This adds the `data-contained-scroll-tokens` API to elements in `ScrollPanel` which allows arbitrary containers of elements with scroll tokens to declare their contained scroll tokens. When jumping to a scroll token inside a container, the `ScrollPanel` will act as if it is scrolling to the container itself, not the child. MELS has been modified such that it exposes the scroll tokens of all events that exist within it.This means "Jump to unread message" will work if the unread event is within a MELS (which is any member event, because even individual member events surrounded by other events are put inside a MELS). --- src/components/structures/MessagePanel.js | 1 - src/components/structures/ScrollPanel.js | 5 +++++ src/components/views/elements/MemberEventListSummary.js | 5 +++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 246f351841..74aec61511 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -354,7 +354,6 @@ module.exports = React.createClass({ {eventTiles} diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index d43e22e2f1..da3b303d89 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -551,6 +551,11 @@ module.exports = React.createClass({ var messages = this.refs.itemlist.children; for (var i = messages.length-1; i >= 0; --i) { var m = messages[i]; + if (m.dataset.containedScrollTokens && + m.dataset.containedScrollTokens.indexOf(scrollToken) !== -1) { + node = m; + break; + } if (!m.dataset.scrollToken) continue; if (m.dataset.scrollToken == scrollToken) { node = m; diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 63bd2a7c39..a24e19577d 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -369,6 +369,7 @@ module.exports = React.createClass({ render: function() { const eventsToRender = this.props.events; + const eventIds = eventsToRender.map(e => e.getId()); const fewEvents = eventsToRender.length < this.props.threshold; const expanded = this.state.expanded || fewEvents; @@ -379,7 +380,7 @@ module.exports = React.createClass({ if (fewEvents) { return ( -
+
{expandedEvents}
); @@ -437,7 +438,7 @@ module.exports = React.createClass({ ); return ( -
+
{toggleButton} {summaryContainer} {expanded ?
 
: null} From fe83a99ab709121d2259b25d1a735babb2a022ee Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 2 May 2017 17:36:59 +0100 Subject: [PATCH 07/11] Update ScrollPanel docs --- src/components/structures/ScrollPanel.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index da3b303d89..7fa320d784 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -50,6 +50,10 @@ if (DEBUG_SCROLL) { * serialise the scroll state, and returned as the 'trackedScrollToken' * attribute by getScrollState(). * + * Child elements that contain elements that have scroll tokens must declare the + * contained scroll tokens using 'data-contained-scroll-tokens`. When scrolling + * to a contained scroll token, the ScrollPanel will scroll to the container. + * * Some notes about the implementation: * * The saved 'scrollState' can exist in one of two states: From 4febc63aeecee7613681749b3aa435cbd25ea17f Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 2 May 2017 17:41:09 +0100 Subject: [PATCH 08/11] Add comment to _scrollToToken --- src/components/structures/ScrollPanel.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 7fa320d784..3f36fac89b 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -555,6 +555,9 @@ module.exports = React.createClass({ var messages = this.refs.itemlist.children; for (var i = messages.length-1; i >= 0; --i) { var m = messages[i]; + // 'data-contained-scroll-tokens' has been set, indicating that a child + // element contains elements that each have a token. Check this array of + // tokens for `scrollToken`. if (m.dataset.containedScrollTokens && m.dataset.containedScrollTokens.indexOf(scrollToken) !== -1) { node = m; From af137f8867dce643232c8edfa92294b8f87b3b89 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 2 May 2017 18:30:46 +0100 Subject: [PATCH 09/11] Validate phone number on login To prevent confusion when accidently inputting mxid or email. Fixes https://github.com/vector-im/riot-web/issues/3637 --- src/components/structures/login/Login.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/components/structures/login/Login.js b/src/components/structures/login/Login.js index 315a0ea242..a3635177e2 100644 --- a/src/components/structures/login/Login.js +++ b/src/components/structures/login/Login.js @@ -23,6 +23,9 @@ import url from 'url'; import sdk from '../../../index'; import Login from '../../../Login'; +// For validating phone numbers without country codes +const PHONE_NUMBER_REGEX = /^[0-9\(\)\-\s]*$/; + /** * A wire component which glues together login UI components and Login logic */ @@ -125,7 +128,16 @@ module.exports = React.createClass({ }, onPhoneNumberChanged: function(phoneNumber) { - this.setState({ phoneNumber: phoneNumber }); + // Validate the phone number entered + if (!PHONE_NUMBER_REGEX.test(phoneNumber)) { + this.setState({ errorText: 'The phone number entered looks invalid' }); + return; + } + + this.setState({ + phoneNumber: phoneNumber, + errorText: null, + }); }, onServerConfigChange: function(config) { From bfa3123f9ba7f3aeab19c7205123c4639f7cd1ea Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 4 May 2017 10:00:13 +0100 Subject: [PATCH 10/11] Combine data-scroll-token and -contained-scroll-tokens - Instead of using one attribute, use one that might just contain one token - Use the first token when tracking a child - Mandate that no commas can be in individual tokens --- src/components/structures/MessagePanel.js | 2 +- src/components/structures/ScrollPanel.js | 41 ++++++++----------- .../views/elements/MemberEventListSummary.js | 6 +-- .../views/rooms/SearchResultTile.js | 2 +- .../components/structures/ScrollPanel-test.js | 2 +- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 74aec61511..d4bf147ad5 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -472,7 +472,7 @@ module.exports = React.createClass({ ret.push(
  • + data-scroll-tokens={scrollToken}> excessHeight) { break; @@ -423,7 +423,8 @@ module.exports = React.createClass({ * scroll. false if we are tracking a particular child. * * string trackedScrollToken: undefined if stuckAtBottom is true; if it is - * false, the data-scroll-token of the child which we are tracking. + * false, the fist token in data-scroll-tokens of the child which we are + * tracking. * * number pixelOffset: undefined if stuckAtBottom is true; if it is false, * the number of pixels the bottom of the tracked child is above the @@ -555,16 +556,10 @@ module.exports = React.createClass({ var messages = this.refs.itemlist.children; for (var i = messages.length-1; i >= 0; --i) { var m = messages[i]; - // 'data-contained-scroll-tokens' has been set, indicating that a child - // element contains elements that each have a token. Check this array of - // tokens for `scrollToken`. - if (m.dataset.containedScrollTokens && - m.dataset.containedScrollTokens.indexOf(scrollToken) !== -1) { - node = m; - break; - } - if (!m.dataset.scrollToken) continue; - if (m.dataset.scrollToken == scrollToken) { + // 'data-scroll-tokens' is a DOMString of comma-separated scroll tokens + // There might only be one scroll token + if (m.dataset.scrollTokens && + m.dataset.scrollTokens.split(',').indexOf(scrollToken) !== -1) { node = m; break; } @@ -580,7 +575,7 @@ module.exports = React.createClass({ var boundingRect = node.getBoundingClientRect(); var scrollDelta = boundingRect.bottom + pixelOffset - wrapperRect.bottom; - debuglog("ScrollPanel: scrolling to token '" + node.dataset.scrollToken + "'+" + + debuglog("ScrollPanel: scrolling to token '" + scrollToken + "'+" + pixelOffset + " (delta: "+scrollDelta+")"); if(scrollDelta != 0) { @@ -603,12 +598,12 @@ module.exports = React.createClass({ for (var i = messages.length-1; i >= 0; --i) { var node = messages[i]; - if (!node.dataset.scrollToken) continue; + if (!node.dataset.scrollTokens) continue; var boundingRect = node.getBoundingClientRect(); newScrollState = { stuckAtBottom: false, - trackedScrollToken: node.dataset.scrollToken, + trackedScrollToken: node.dataset.scrollTokens.split(',')[0], pixelOffset: wrapperRect.bottom - boundingRect.bottom, }; // If the bottom of the panel intersects the ClientRect of node, use this node @@ -620,7 +615,7 @@ module.exports = React.createClass({ break; } } - // This is only false if there were no nodes with `node.dataset.scrollToken` set. + // 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); diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index a24e19577d..ae8678894d 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -369,7 +369,7 @@ module.exports = React.createClass({ render: function() { const eventsToRender = this.props.events; - const eventIds = eventsToRender.map(e => e.getId()); + const eventIds = eventsToRender.map(e => e.getId()).join(','); const fewEvents = eventsToRender.length < this.props.threshold; const expanded = this.state.expanded || fewEvents; @@ -380,7 +380,7 @@ module.exports = React.createClass({ if (fewEvents) { return ( -
    +
    {expandedEvents}
    ); @@ -438,7 +438,7 @@ module.exports = React.createClass({ ); return ( -
    +
    {toggleButton} {summaryContainer} {expanded ?
     
    : null} diff --git a/src/components/views/rooms/SearchResultTile.js b/src/components/views/rooms/SearchResultTile.js index 7fac244481..1aba7c9196 100644 --- a/src/components/views/rooms/SearchResultTile.js +++ b/src/components/views/rooms/SearchResultTile.js @@ -60,7 +60,7 @@ module.exports = React.createClass({ } } return ( -
  • +
  • {ret}
    • ); }, diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js index eacaeb5fb4..7ecb74be6f 100644 --- a/test/components/structures/ScrollPanel-test.js +++ b/test/components/structures/ScrollPanel-test.js @@ -115,7 +115,7 @@ var Tester = React.createClass({ // // there is an extra 50 pixels of margin at the bottom. return ( -
  • +
  • {key} From 6d9a1f047d1a2a82d3d0ab9596a30eafafb5c626 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 4 May 2017 13:03:04 +0100 Subject: [PATCH 11/11] Typo --- src/components/structures/ScrollPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 50951312e7..a652bcc827 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -423,7 +423,7 @@ module.exports = React.createClass({ * scroll. false if we are tracking a particular child. * * string trackedScrollToken: undefined if stuckAtBottom is true; if it is - * false, the fist token in data-scroll-tokens of the child which we are + * false, the first token in data-scroll-tokens of the child which we are * tracking. * * number pixelOffset: undefined if stuckAtBottom is true; if it is false,