From 842e19280de6e6036be99f246bf380947482dcd3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 1 Apr 2019 11:49:29 -0600 Subject: [PATCH 1/4] Apply changes to breadcrumbs setting live --- src/components/structures/LeftPanel.js | 23 ++++++++++++++++++- src/i18n/strings/en_EN.json | 2 +- src/settings/Settings.js | 2 +- .../handlers/DeviceSettingsHandler.js | 2 +- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/components/structures/LeftPanel.js b/src/components/structures/LeftPanel.js index 95b57a0ca5..88b6a13a5d 100644 --- a/src/components/structures/LeftPanel.js +++ b/src/components/structures/LeftPanel.js @@ -45,11 +45,22 @@ const LeftPanel = React.createClass({ getInitialState: function() { return { searchFilter: '', + breadcrumbs: false, }; }, componentWillMount: function() { this.focusedElement = null; + + this._settingWatchRef = SettingsStore.watchSetting( + "feature_room_breadcrumbs", null, + this._onBreadcrumbsChanged); + + this.setState({breadcrumbs: SettingsStore.isFeatureEnabled("feature_room_breadcrumbs")}); + }, + + componentWillUnmount: function() { + SettingsStore.unwatchSetting(this._settingWatchRef); }, shouldComponentUpdate: function(nextProps, nextState) { @@ -73,6 +84,16 @@ const LeftPanel = React.createClass({ return false; }, + _onBreadcrumbsChanged: function(settingName, roomId, level, valueAtLevel, value) { + // Features are only possible at a single level, so we can get away with using valueAtLevel. + // The SettingsStore runs on the same tick as the update, so `value` will be wrong. + this.setState({breadcrumbs: valueAtLevel}); + + // For some reason the setState doesn't trigger a render of the component, so force one. + // Probably has to do with the change happening outside of a change detector cycle. + this.forceUpdate(); + }, + _onFocus: function(ev) { this.focusedElement = ev.target; }, @@ -220,7 +241,7 @@ const LeftPanel = React.createClass({ collapsed={this.props.collapsed} />); let breadcrumbs; - if (SettingsStore.isFeatureEnabled("feature_room_breadcrumbs")) { + if (this.state.breadcrumbs) { breadcrumbs = (); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 5b95c7ffe3..358f78b75f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -288,7 +288,7 @@ "Failed to join room": "Failed to join room", "Message Pinning": "Message Pinning", "Custom user status messages": "Custom user status messages", - "Show recent room avatars above the room list (refresh to apply changes)": "Show recent room avatars above the room list (refresh to apply changes)", + "Show recent room avatars above the room list": "Show recent room avatars above the room list", "Group & filter rooms by custom tags (refresh to apply changes)": "Group & filter rooms by custom tags (refresh to apply changes)", "Render simple counters in room header": "Render simple counters in room header", "Enable Emoji suggestions while typing": "Enable Emoji suggestions while typing", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 6e17ffbbd7..71a81ae4db 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -102,7 +102,7 @@ export const SETTINGS = { }, "feature_room_breadcrumbs": { isFeature: true, - displayName: _td("Show recent room avatars above the room list (refresh to apply changes)"), + displayName: _td("Show recent room avatars above the room list"), supportedLevels: LEVELS_FEATURE, default: false, }, diff --git a/src/settings/handlers/DeviceSettingsHandler.js b/src/settings/handlers/DeviceSettingsHandler.js index bca7679a47..b5c4382768 100644 --- a/src/settings/handlers/DeviceSettingsHandler.js +++ b/src/settings/handlers/DeviceSettingsHandler.js @@ -127,6 +127,6 @@ export default class DeviceSettingsHandler extends SettingsHandler { _writeFeature(featureName, enabled) { localStorage.setItem("mx_labs_feature_" + featureName, enabled); - this._watchers.notifyUpdate(featureName, null, enabled); + this._watchers.notifyUpdate(featureName, null, SettingLevel.DEVICE, enabled); } } From 035f6b14c0ea5977e2573d584d05783335593d96 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 4 Apr 2019 14:17:15 -0600 Subject: [PATCH 2/4] Add telemetry for how many people use breadcrumbs Fixes https://github.com/vector-im/riot-web/issues/9207 --- src/Analytics.js | 10 ++++++++++ src/components/structures/LeftPanel.js | 12 +++++++++--- src/components/views/rooms/RoomBreadcrumbs.js | 6 ++++-- src/i18n/strings/en_EN.json | 3 ++- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/Analytics.js b/src/Analytics.js index 1bff3bdf7c..3e208ad6bd 100644 --- a/src/Analytics.js +++ b/src/Analytics.js @@ -84,6 +84,11 @@ const customVariables = { expl: _td('Whether or not you\'re using the Richtext mode of the Rich Text Editor'), example: 'off', }, + 'Breadcrumbs': { + id: 9, + expl: _td("Whether or not you're using the 'breadcrumbs' feature (avatars above the room list)"), + example: 'disabled', + }, 'Homeserver URL': { id: 7, expl: _td('Your homeserver\'s URL'), @@ -234,6 +239,11 @@ class Analytics { this._setVisitVariable('RTE: Uses Richtext Mode', state ? 'on' : 'off'); } + setBreadcrumbs(state) { + if (this.disabled) return; + this._setVisitVariable('Breadcrumbs', state ? 'enabled' : 'disabled'); + } + showDetailsModal() { let rows = []; if (window.Piwik) { diff --git a/src/components/structures/LeftPanel.js b/src/components/structures/LeftPanel.js index 88b6a13a5d..ccac053776 100644 --- a/src/components/structures/LeftPanel.js +++ b/src/components/structures/LeftPanel.js @@ -27,6 +27,7 @@ import VectorConferenceHandler from '../../VectorConferenceHandler'; import TagPanelButtons from './TagPanelButtons'; import SettingsStore from '../../settings/SettingsStore'; import {_t} from "../../languageHandler"; +import Analytics from "../../Analytics"; const LeftPanel = React.createClass({ @@ -53,10 +54,11 @@ const LeftPanel = React.createClass({ this.focusedElement = null; this._settingWatchRef = SettingsStore.watchSetting( - "feature_room_breadcrumbs", null, - this._onBreadcrumbsChanged); + "feature_room_breadcrumbs", null, this._onBreadcrumbsChanged); - this.setState({breadcrumbs: SettingsStore.isFeatureEnabled("feature_room_breadcrumbs")}); + const useBreadcrumbs = SettingsStore.isFeatureEnabled("feature_room_breadcrumbs"); + Analytics.setBreadcrumbs(useBreadcrumbs); + this.setState({breadcrumbs: useBreadcrumbs}); }, componentWillUnmount: function() { @@ -84,6 +86,10 @@ const LeftPanel = React.createClass({ return false; }, + componentDidUpdate(newProps, newState) { + Analytics.setBreadcrumbs(newState.breadcrumbs); + }, + _onBreadcrumbsChanged: function(settingName, roomId, level, valueAtLevel, value) { // Features are only possible at a single level, so we can get away with using valueAtLevel. // The SettingsStore runs on the same tick as the update, so `value` will be wrong. diff --git a/src/components/views/rooms/RoomBreadcrumbs.js b/src/components/views/rooms/RoomBreadcrumbs.js index 314b2912cd..11fd9545fc 100644 --- a/src/components/views/rooms/RoomBreadcrumbs.js +++ b/src/components/views/rooms/RoomBreadcrumbs.js @@ -24,6 +24,7 @@ import classNames from 'classnames'; import sdk from "../../../index"; import * as RoomNotifs from '../../../RoomNotifs'; import * as FormattingUtils from "../../../utils/FormattingUtils"; +import Analytics from "../../../Analytics"; const MAX_ROOMS = 20; @@ -175,7 +176,8 @@ export default class RoomBreadcrumbs extends React.Component { this.setState({rooms}); } - _viewRoom(room) { + _viewRoom(room, index) { + Analytics.trackEvent("Breadcrumbs", "click_node", index); dis.dispatch({action: "view_room", room_id: room.roomId}); } @@ -233,7 +235,7 @@ export default class RoomBreadcrumbs extends React.Component { } return ( - this._viewRoom(r.room)} + this._viewRoom(r.room, i)} onMouseEnter={() => this._onMouseEnter(r.room)} onMouseLeave={() => this._onMouseLeave(r.room)}> {badge} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 95dfaeabcb..45abb3222a 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -8,6 +8,7 @@ "Your language of choice": "Your language of choice", "Which officially provided instance you are using, if any": "Which officially provided instance you are using, if any", "Whether or not you're using the Richtext mode of the Rich Text Editor": "Whether or not you're using the Richtext mode of the Rich Text Editor", + "Whether or not you're using the 'breadcrumbs' feature (avatars above the room list)": "Whether or not you're using the 'breadcrumbs' feature (avatars above the room list)", "Your homeserver's URL": "Your homeserver's URL", "Your identity server's URL": "Your identity server's URL", "e.g. %(exampleValue)s": "e.g. %(exampleValue)s", @@ -729,8 +730,8 @@ "Send a reply (unencrypted)…": "Send a reply (unencrypted)…", "Send an encrypted message…": "Send an encrypted message…", "Send a message (unencrypted)…": "Send a message (unencrypted)…", - "This room has been replaced and is no longer active.": "This room has been replaced and is no longer active.", "The conversation continues here.": "The conversation continues here.", + "This room has been replaced and is no longer active.": "This room has been replaced and is no longer active.", "You do not have permission to post to this room": "You do not have permission to post to this room", "Markdown is disabled": "Markdown is disabled", "Hide Text Formatting Toolbar": "Hide Text Formatting Toolbar", From edc24c6a914e95d8394838f0f867f37945379a0f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 4 Apr 2019 15:08:38 -0600 Subject: [PATCH 3/4] Move import to avoid future merge conflicts --- src/components/views/rooms/RoomBreadcrumbs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomBreadcrumbs.js b/src/components/views/rooms/RoomBreadcrumbs.js index 11fd9545fc..fffeed3b17 100644 --- a/src/components/views/rooms/RoomBreadcrumbs.js +++ b/src/components/views/rooms/RoomBreadcrumbs.js @@ -22,9 +22,9 @@ import AccessibleButton from '../elements/AccessibleButton'; import RoomAvatar from '../avatars/RoomAvatar'; import classNames from 'classnames'; import sdk from "../../../index"; +import Analytics from "../../../Analytics"; import * as RoomNotifs from '../../../RoomNotifs'; import * as FormattingUtils from "../../../utils/FormattingUtils"; -import Analytics from "../../../Analytics"; const MAX_ROOMS = 20; From 1c41c3dd63e4fcb476ff09609e218b8511535d1c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 5 Apr 2019 09:40:21 -0600 Subject: [PATCH 4/4] Only update analytics when there's a change Also, the arguments are the previous state, not the new state. --- src/components/structures/LeftPanel.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/structures/LeftPanel.js b/src/components/structures/LeftPanel.js index ccac053776..21c41125a4 100644 --- a/src/components/structures/LeftPanel.js +++ b/src/components/structures/LeftPanel.js @@ -86,8 +86,10 @@ const LeftPanel = React.createClass({ return false; }, - componentDidUpdate(newProps, newState) { - Analytics.setBreadcrumbs(newState.breadcrumbs); + componentDidUpdate(prevProps, prevState) { + if (prevState.breadcrumbs !== this.state.breadcrumbs) { + Analytics.setBreadcrumbs(this.state.breadcrumbs); + } }, _onBreadcrumbsChanged: function(settingName, roomId, level, valueAtLevel, value) {