From 7905e6cc3b9933153e5dc73c2d1b9d24c51308a3 Mon Sep 17 00:00:00 2001 From: Aaron Raimist Date: Tue, 26 Nov 2019 14:31:11 -0600 Subject: [PATCH 1/9] Add a link to the labs feature documentation Signed-off-by: Aaron Raimist --- .../views/settings/tabs/user/LabsUserSettingsTab.js | 9 +++++++++ src/i18n/strings/en_EN.json | 1 + 2 files changed, 10 insertions(+) diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js index 07a2bf722a..71a3e5f1d2 100644 --- a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js @@ -49,6 +49,15 @@ export default class LabsUserSettingsTab extends React.Component { return (
{_t("Labs")}
+
+ { + _t('These are experimental features. For more information on what ' + + 'these options do see the documentation.', {}, { + 'a': (sub) => {sub}, + }) + } +
{flags} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 9136f432dd..9522655698 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -641,6 +641,7 @@ "Access Token:": "Access Token:", "click to reveal": "click to reveal", "Labs": "Labs", + "These are experimental features. For more information on what these options do see the documentation.": "These are experimental features. For more information on what these options do see the documentation.", "Ignored/Blocked": "Ignored/Blocked", "Error adding ignored user/server": "Error adding ignored user/server", "Something went wrong. Please try again or view your console for hints.": "Something went wrong. Please try again or view your console for hints.", From 98c47265c9d7c9f16a27946fee8dab946158fce4 Mon Sep 17 00:00:00 2001 From: Aaron Raimist Date: Tue, 26 Nov 2019 15:00:56 -0600 Subject: [PATCH 2/9] Fix indentation Signed-off-by: Aaron Raimist --- .../views/settings/tabs/user/LabsUserSettingsTab.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js index 71a3e5f1d2..4232cc90fb 100644 --- a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js @@ -53,8 +53,10 @@ export default class LabsUserSettingsTab extends React.Component { { _t('These are experimental features. For more information on what ' + 'these options do see the documentation.', {}, { - 'a': (sub) => {sub}, + 'a': (sub) => { + return {sub}; + }, }) }
From 37ec7e6f7edea88feaa5f94f1d1868363f7f5c51 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 28 Nov 2019 13:40:28 -0700 Subject: [PATCH 3/9] Ensure the settings page accurately represents theme choices Fixes https://github.com/vector-im/riot-web/issues/11518 This also fixes a bug where the the theme logic can incorrectly get stuck in the light theme - the ThemeController was overriding *all* values, not just supposed themes. Null values aren't overridden so that the various theme logic bits don't assume the user has chosen the light theme explicitly. --- .../tabs/user/GeneralUserSettingsTab.js | 49 ++++++++++++++++--- src/settings/controllers/ThemeController.js | 2 + src/theme.js | 2 + 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index b518f7c81b..cae4b19891 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -49,8 +49,6 @@ export default class GeneralUserSettingsTab extends React.Component { this.state = { language: languageHandler.getCurrentLanguage(), - theme: SettingsStore.getValueAt(SettingLevel.ACCOUNT, "theme"), - useSystemTheme: SettingsStore.getValueAt(SettingLevel.DEVICE, "use_system_theme"), haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl()), serverSupportsSeparateAddAndBind: null, idServerHasUnsignedTerms: false, @@ -62,6 +60,7 @@ export default class GeneralUserSettingsTab extends React.Component { }, emails: [], msisdns: [], + ...this._calculateThemeState(), }; this.dispatcherRef = dis.register(this._onAction); @@ -80,6 +79,39 @@ export default class GeneralUserSettingsTab extends React.Component { dis.unregister(this.dispatcherRef); } + _calculateThemeState() { + // We have to mirror the logic from ThemeWatcher.getEffectiveTheme so we + // show the right values for things. + + const themeChoice = SettingsStore.getValueAt(SettingLevel.ACCOUNT, "theme"); + const systemThemeExplicit = SettingsStore.getValueAt( + SettingLevel.DEVICE, "use_system_theme", null, false, true); + const themeExplicit = SettingsStore.getValueAt( + SettingLevel.DEVICE, "theme", null, false, true); + + // If the user has enabled system theme matching, use that. + if (systemThemeExplicit) { + return { + theme: themeChoice, + useSystemTheme: true, + }; + } + + // If the user has set a theme explicitly, use that (no system theme matching) + if (themeExplicit) { + return { + theme: themeChoice, + useSystemTheme: false, + }; + } + + // Otherwise assume the defaults for the settings + return { + theme: themeChoice, + useSystemTheme: SettingsStore.getValueAt(SettingLevel.DEVICE, "use_system_theme"), + }; + } + _onAction = (payload) => { if (payload.action === 'id_server_changed') { this.setState({haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl())}); @@ -89,11 +121,11 @@ export default class GeneralUserSettingsTab extends React.Component { _onEmailsChange = (emails) => { this.setState({ emails }); - } + }; _onMsisdnsChange = (msisdns) => { this.setState({ msisdns }); - } + }; async _getThreepidState() { const cli = MatrixClientPeg.get(); @@ -193,9 +225,9 @@ export default class GeneralUserSettingsTab extends React.Component { _onUseSystemThemeChanged = (checked) => { this.setState({useSystemTheme: checked}); + SettingsStore.setValue("use_system_theme", null, SettingLevel.DEVICE, checked); dis.dispatch({action: 'recheck_theme'}); - } - + }; _onPasswordChangeError = (err) => { // TODO: Figure out a design that doesn't involve replacing the current dialog @@ -307,12 +339,15 @@ export default class GeneralUserSettingsTab extends React.Component { _renderThemeSection() { const SettingsFlag = sdk.getComponent("views.elements.SettingsFlag"); + const LabelledToggleSwitch = sdk.getComponent("views.elements.LabelledToggleSwitch"); const themeWatcher = new ThemeWatcher(); let systemThemeSection; if (themeWatcher.isSystemThemeSupported()) { systemThemeSection =
-
; diff --git a/src/settings/controllers/ThemeController.js b/src/settings/controllers/ThemeController.js index a15b4e78cd..aae43673bf 100644 --- a/src/settings/controllers/ThemeController.js +++ b/src/settings/controllers/ThemeController.js @@ -20,6 +20,8 @@ import {DEFAULT_THEME, enumerateThemes} from "../../theme"; export default class ThemeController extends SettingController { getValueOverride(level, roomId, calculatedValue, calculatedAtLevel) { + if (!calculatedValue) return null; // Don't override null themes + const themes = enumerateThemes(); // Override in case some no longer supported theme is stored here if (!themes[calculatedValue]) { diff --git a/src/theme.js b/src/theme.js index 3f50f8ba88..1ba87a2048 100644 --- a/src/theme.js +++ b/src/theme.js @@ -80,6 +80,8 @@ export class ThemeWatcher { } getEffectiveTheme() { + // Dev note: Much of this logic is replicated in the GeneralUserSettingsTab + // If the user has specifically enabled the system matching option (excluding default), // then use that over anything else. We pick the lowest possible level for the setting // to ensure the ordering otherwise works. From 24843cf25ed310653574f688ff5078eeb1357e59 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 28 Nov 2019 19:13:55 -0700 Subject: [PATCH 4/9] Convert Velociraptor to a class --- src/Velociraptor.js | 88 ++++++++++++++-------------------- src/stores/ReadReceiptStore.js | 0 2 files changed, 37 insertions(+), 51 deletions(-) create mode 100644 src/stores/ReadReceiptStore.js diff --git a/src/Velociraptor.js b/src/Velociraptor.js index b7a2d7fb40..245ca6648b 100644 --- a/src/Velociraptor.js +++ b/src/Velociraptor.js @@ -1,7 +1,6 @@ const React = require('react'); const ReactDom = require('react-dom'); import PropTypes from 'prop-types'; -import createReactClass from 'create-react-class'; const Velocity = require('velocity-animate'); /** @@ -11,10 +10,8 @@ const Velocity = require('velocity-animate'); * from DOM order. This makes it a lot simpler and lighter: if you need fully * automatic positional animation, look at react-shuffle or similar libraries. */ -module.exports = createReactClass({ - displayName: 'Velociraptor', - - propTypes: { +export default class Velociraptor extends React.Component { + static propTypes = { // either a list of child nodes, or a single child. children: PropTypes.any, @@ -26,82 +23,71 @@ module.exports = createReactClass({ // a list of transition options from the corresponding startStyle enterTransitionOpts: PropTypes.array, - }, + }; - getDefaultProps: function() { - return { - startStyles: [], - enterTransitionOpts: [], - }; - }, + static defaultProps = { + startStyles: [], + enterTransitionOpts: [], + }; + + constructor(props) { + super(props); - componentWillMount: function() { this.nodes = {}; this._updateChildren(this.props.children); - }, + } - componentWillReceiveProps: function(nextProps) { - this._updateChildren(nextProps.children); - }, + componentDidUpdate() { + this._updateChildren(this.props.children); + } - /** - * update `this.children` according to the new list of children given - */ - _updateChildren: function(newChildren) { - const self = this; + _updateChildren(newChildren) { const oldChildren = this.children || {}; this.children = {}; - React.Children.toArray(newChildren).forEach(function(c) { + React.Children.toArray(newChildren).forEach((c) => { if (oldChildren[c.key]) { const old = oldChildren[c.key]; - const oldNode = ReactDom.findDOMNode(self.nodes[old.key]); + const oldNode = ReactDom.findDOMNode(this.nodes[old.key]); - if (oldNode && oldNode.style.left != c.props.style.left) { - Velocity(oldNode, { left: c.props.style.left }, self.props.transition).then(function() { + if (oldNode && oldNode.style.left !== c.props.style.left) { + Velocity(oldNode, { left: c.props.style.left }, this.props.transition).then(() => { // special case visibility because it's nonsensical to animate an invisible element // so we always hidden->visible pre-transition and visible->hidden after - if (oldNode.style.visibility == 'visible' && c.props.style.visibility == 'hidden') { + if (oldNode.style.visibility === 'visible' && c.props.style.visibility === 'hidden') { oldNode.style.visibility = c.props.style.visibility; } }); //console.log("translation: "+oldNode.style.left+" -> "+c.props.style.left); } - if (oldNode && oldNode.style.visibility == 'hidden' && c.props.style.visibility == 'visible') { + if (oldNode && oldNode.style.visibility === 'hidden' && c.props.style.visibility === 'visible') { oldNode.style.visibility = c.props.style.visibility; } // clone the old element with the props (and children) of the new element // so prop updates are still received by the children. - self.children[c.key] = React.cloneElement(old, c.props, c.props.children); + this.children[c.key] = React.cloneElement(old, c.props, c.props.children); } else { // new element. If we have a startStyle, use that as the style and go through // the enter animations const newProps = {}; const restingStyle = c.props.style; - const startStyles = self.props.startStyles; + const startStyles = this.props.startStyles; if (startStyles.length > 0) { const startStyle = startStyles[0]; newProps.style = startStyle; // console.log("mounted@startstyle0: "+JSON.stringify(startStyle)); } - newProps.ref = ((n) => self._collectNode( + newProps.ref = ((n) => this._collectNode( c.key, n, restingStyle, )); - self.children[c.key] = React.cloneElement(c, newProps); + this.children[c.key] = React.cloneElement(c, newProps); } }); - }, + } - /** - * called when a child element is mounted/unmounted - * - * @param {string} k key of the child - * @param {null|Object} node On mount: React node. On unmount: null - * @param {Object} restingStyle final style - */ - _collectNode: function(k, node, restingStyle) { + _collectNode(k, node, restingStyle) { if ( node && this.nodes[k] === undefined && @@ -125,12 +111,12 @@ module.exports = createReactClass({ // and then we animate to the resting state Velocity(domNode, restingStyle, - transitionOpts[i-1]) - .then(() => { - // once we've reached the resting state, hide the element if - // appropriate - domNode.style.visibility = restingStyle.visibility; - }); + transitionOpts[i-1]) + .then(() => { + // once we've reached the resting state, hide the element if + // appropriate + domNode.style.visibility = restingStyle.visibility; + }); /* console.log("enter:", @@ -153,13 +139,13 @@ module.exports = createReactClass({ if (domNode) Velocity.Utilities.removeData(domNode); } this.nodes[k] = node; - }, + } - render: function() { + render() { return ( { Object.values(this.children) } ); - }, -}); + } +} diff --git a/src/stores/ReadReceiptStore.js b/src/stores/ReadReceiptStore.js new file mode 100644 index 0000000000..e69de29bb2 From 36f9fab47443c5ac571f37ee9081a6cdb7e85a6a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 28 Nov 2019 19:45:37 -0700 Subject: [PATCH 5/9] Ensure read receipts end up with a valid reference to checkUnmounting Fixes https://github.com/vector-im/riot-web/issues/11496 Fixes https://github.com/vector-im/riot-web/issues/11385 Fixes https://github.com/vector-im/riot-web/issues/10007 Fixes https://github.com/vector-im/riot-web/issues/9769 React does (kinda) bind `this._isUnmounting` for us in the context of the EventTile, but the EventTile then passes the function straight through to the ReadReceiptMarker component, which then binds it in the context of EventTile. This results in `this._mounted` being falsey all the time, preventing the ReadReceiptMarker from hitting the code where it updates rrInfo in its unmount. The velocity stuff is smart enough to realize that it has a read receipt and shuffles everything over by one, but when it goes to check the starting height (which will be null/undefined because the RRMarker didn't update it) it assumes it has never seen the receipt before and appends it again - this is what causes some holes/stacking. By forcefully binding the `this._isUnmounting` function we ensure that the `this._mounted` variable is correctly referenced in the context of the MessagePanel, allowing the RRMarker to update its position, and therefore allowing the velocity behaviour to be consistent. --- src/components/structures/MessagePanel.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index d1cc1b7caf..c6f218377a 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -693,6 +693,10 @@ export default class MessagePanel extends React.Component { const readReceipts = this._readReceiptsByEvent[eventId]; + // Dev note: `this._isUnmounting.bind(this)` is important - it ensures that + // the function is run in the context of this class and not EventTile, therefore + // ensuring the right `this._mounted` variable is used by read receipts (which + // don't update their position if we, the MessagePanel, is unmounting). ret.push(
  • Date: Thu, 28 Nov 2019 22:22:38 -0600 Subject: [PATCH 6/9] Improve wording Signed-off-by: Aaron Raimist --- .../views/settings/tabs/user/LabsUserSettingsTab.js | 4 ++-- src/i18n/strings/en_EN.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js index 4232cc90fb..5f7d75c5c3 100644 --- a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js @@ -51,8 +51,8 @@ export default class LabsUserSettingsTab extends React.Component {
    {_t("Labs")}
    { - _t('These are experimental features. For more information on what ' + - 'these options do see the documentation.', {}, { + _t('Customise your experience with experimental labs features. ' + + 'Learn more.', {}, { 'a': (sub) => { return {sub}; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 9522655698..c9bc2d6f4b 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -641,7 +641,7 @@ "Access Token:": "Access Token:", "click to reveal": "click to reveal", "Labs": "Labs", - "These are experimental features. For more information on what these options do see the documentation.": "These are experimental features. For more information on what these options do see the documentation.", + "Customise your experience with experimental labs features. Learn more.": "Customise your experience with experimental labs features. Learn more.", "Ignored/Blocked": "Ignored/Blocked", "Error adding ignored user/server": "Error adding ignored user/server", "Something went wrong. Please try again or view your console for hints.": "Something went wrong. Please try again or view your console for hints.", From f4b1c4f360e5b7b4912929292fb54e4c78274d8c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 29 Nov 2019 15:13:46 -0700 Subject: [PATCH 7/9] Remove useless file --- src/stores/ReadReceiptStore.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/stores/ReadReceiptStore.js diff --git a/src/stores/ReadReceiptStore.js b/src/stores/ReadReceiptStore.js deleted file mode 100644 index e69de29bb2..0000000000 From 3ad4b0fb64e0d936cd5e67dc30c5124325e1faa1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 2 Dec 2019 10:01:08 +0000 Subject: [PATCH 8/9] Do not trap Key ContextMenu into composer for keyboard a11y Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Keyboard.js | 1 + src/components/structures/LoggedInView.js | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/Keyboard.js b/src/Keyboard.js index f63956777f..453ddab1e2 100644 --- a/src/Keyboard.js +++ b/src/Keyboard.js @@ -78,6 +78,7 @@ export const Key = { CONTROL: "Control", META: "Meta", SHIFT: "Shift", + CONTEXT_MENU: "ContextMenu", LESS_THAN: "<", GREATER_THAN: ">", diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index d071ba1d79..ae71af1a85 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -401,6 +401,11 @@ const LoggedInView = createReactClass({ const isClickShortcut = ev.target !== document.body && (ev.key === Key.SPACE || ev.key === Key.ENTER); + // Do not capture the context menu key to improve keyboard accessibility + if (ev.key === Key.CONTEXT_MENU) { + return; + } + // XXX: Remove after CIDER replaces Slate completely: https://github.com/vector-im/riot-web/issues/11036 // If using Slate, consume the Backspace without first focusing as it causes an implosion if (ev.key === Key.BACKSPACE && !SettingsStore.getValue("useCiderComposer")) { From 7a3b8a522fd249b5aa2126730aacf79322eb9747 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 2 Dec 2019 10:18:02 +0000 Subject: [PATCH 9/9] Make EmojiPicker filtering case-insensitive Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/emojipicker/EmojiPicker.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/emojipicker/EmojiPicker.js b/src/components/views/emojipicker/EmojiPicker.js index 9fe8b4c81e..0ec11c2b38 100644 --- a/src/components/views/emojipicker/EmojiPicker.js +++ b/src/components/views/emojipicker/EmojiPicker.js @@ -62,7 +62,7 @@ EMOJIBASE.forEach(emoji => { DATA_BY_CATEGORY[categoryId].push(emoji); } // This is used as the string to match the query against when filtering emojis. - emoji.filterString = `${emoji.annotation}\n${emoji.shortcodes.join('\n')}}\n${emoji.emoticon || ''}`; + emoji.filterString = `${emoji.annotation}\n${emoji.shortcodes.join('\n')}}\n${emoji.emoticon || ''}`.toLowerCase(); }); export const CATEGORY_HEADER_HEIGHT = 22; @@ -201,6 +201,7 @@ class EmojiPicker extends React.Component { } onChangeFilter(filter) { + filter = filter.toLowerCase(); // filter is case insensitive stored lower-case for (const cat of this.categories) { let emojis; // If the new filter string includes the old filter string, we don't have to re-filter the whole dataset.