From 96087d61f689b97217548cb2a47a69e51b69e58b Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 17 Aug 2020 13:12:18 -0600 Subject: [PATCH 1/7] Convert feature setting usages to regular settings --- src/FromWidgetPostMessageApi.js | 2 +- src/components/structures/LeftPanel.tsx | 2 +- .../avatars/MemberStatusMessageAvatar.js | 4 +- .../views/context_menus/MessageContextMenu.js | 2 +- .../views/dialogs/RoomSettingsDialog.js | 2 +- .../views/dialogs/UserSettingsDialog.js | 2 +- src/components/views/elements/AppTile.js | 2 +- .../views/elements/InlineSpinner.js | 2 +- .../views/elements/ManageIntegsButton.js | 2 +- src/components/views/elements/Spinner.js | 2 +- src/components/views/messages/MessageEvent.js | 2 +- src/components/views/right_panel/UserInfo.js | 2 +- src/components/views/rooms/AppsDrawer.js | 2 +- src/components/views/rooms/AuxPanel.js | 6 +- src/components/views/rooms/MemberTile.js | 4 +- src/components/views/rooms/RoomHeader.js | 2 +- src/components/views/rooms/Stickerpicker.js | 2 +- .../tabs/user/AppearanceUserSettingsTab.tsx | 2 +- .../settings/tabs/user/LabsUserSettingsTab.js | 2 +- src/hooks/useSettings.ts | 4 +- src/rageshake/submit-rageshake.ts | 2 +- src/settings/SettingsStore.ts | 71 ------------------- src/stores/CustomRoomTagStore.js | 2 +- src/stores/room-list/RoomListStore.ts | 2 +- .../previews/ReactionEventPreview.ts | 4 +- 25 files changed, 30 insertions(+), 101 deletions(-) diff --git a/src/FromWidgetPostMessageApi.js b/src/FromWidgetPostMessageApi.js index 1b4aa19ebf..d5d7c08d50 100644 --- a/src/FromWidgetPostMessageApi.js +++ b/src/FromWidgetPostMessageApi.js @@ -197,7 +197,7 @@ export default class FromWidgetPostMessageApi { const integId = (data && data.integId) ? data.integId : null; // TODO: Open the right integration manager for the widget - if (SettingsStore.isFeatureEnabled("feature_many_integration_managers")) { + if (SettingsStore.getValue("feature_many_integration_managers")) { IntegrationManagers.sharedInstance().openAll( MatrixClientPeg.get().getRoom(RoomViewStore.getRoomId()), `type_${integType}`, diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index bc17bbe23f..a0d7f3d9ea 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -377,7 +377,7 @@ export default class LeftPanel extends React.Component { const tagPanel = !this.state.showTagPanel ? null : (
- {SettingsStore.isFeatureEnabled("feature_custom_tags") ? : null} + {SettingsStore.getValue("feature_custom_tags") ? : null}
); diff --git a/src/components/views/avatars/MemberStatusMessageAvatar.js b/src/components/views/avatars/MemberStatusMessageAvatar.js index eef3f86d9a..d5d927106c 100644 --- a/src/components/views/avatars/MemberStatusMessageAvatar.js +++ b/src/components/views/avatars/MemberStatusMessageAvatar.js @@ -53,7 +53,7 @@ export default class MemberStatusMessageAvatar extends React.Component { if (this.props.member.userId !== MatrixClientPeg.get().getUserId()) { throw new Error("Cannot use MemberStatusMessageAvatar on anyone but the logged in user"); } - if (!SettingsStore.isFeatureEnabled("feature_custom_status")) { + if (!SettingsStore.getValue("feature_custom_status")) { return; } const { user } = this.props.member; @@ -105,7 +105,7 @@ export default class MemberStatusMessageAvatar extends React.Component { resizeMethod={this.props.resizeMethod} />; - if (!SettingsStore.isFeatureEnabled("feature_custom_status")) { + if (!SettingsStore.getValue("feature_custom_status")) { return avatar; } diff --git a/src/components/views/context_menus/MessageContextMenu.js b/src/components/views/context_menus/MessageContextMenu.js index 59e3d4c230..6fa54058a0 100644 --- a/src/components/views/context_menus/MessageContextMenu.js +++ b/src/components/views/context_menus/MessageContextMenu.js @@ -81,7 +81,7 @@ export default createReactClass({ let canPin = room.currentState.mayClientSendStateEvent('m.room.pinned_events', cli); // HACK: Intentionally say we can't pin if the user doesn't want to use the functionality - if (!SettingsStore.isFeatureEnabled("feature_pinning")) canPin = false; + if (!SettingsStore.getValue("feature_pinning")) canPin = false; this.setState({canRedact, canPin}); }, diff --git a/src/components/views/dialogs/RoomSettingsDialog.js b/src/components/views/dialogs/RoomSettingsDialog.js index 7ad1001f75..613708e436 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.js +++ b/src/components/views/dialogs/RoomSettingsDialog.js @@ -87,7 +87,7 @@ export default class RoomSettingsDialog extends React.Component { , )); - if (SettingsStore.isFeatureEnabled("feature_bridge_state")) { + if (SettingsStore.getValue("feature_bridge_state")) { tabs.push(new Tab( ROOM_BRIDGES_TAB, _td("Bridges"), diff --git a/src/components/views/dialogs/UserSettingsDialog.js b/src/components/views/dialogs/UserSettingsDialog.js index 1f1a8d1523..820de713c1 100644 --- a/src/components/views/dialogs/UserSettingsDialog.js +++ b/src/components/views/dialogs/UserSettingsDialog.js @@ -54,7 +54,7 @@ export default class UserSettingsDialog extends React.Component { super(); this.state = { - mjolnirEnabled: SettingsStore.isFeatureEnabled("feature_mjolnir"), + mjolnirEnabled: SettingsStore.getValue("feature_mjolnir"), }; } diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 75946f19c1..a52dea3e0a 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -311,7 +311,7 @@ export default class AppTile extends React.Component { this.props.onEditClick(); } else { // TODO: Open the right manager for the widget - if (SettingsStore.isFeatureEnabled("feature_many_integration_managers")) { + if (SettingsStore.getValue("feature_many_integration_managers")) { IntegrationManagers.sharedInstance().openAll( this.props.room, 'type_' + this.props.app.type, diff --git a/src/components/views/elements/InlineSpinner.js b/src/components/views/elements/InlineSpinner.js index 89b5e6f19d..ce3c738f3b 100644 --- a/src/components/views/elements/InlineSpinner.js +++ b/src/components/views/elements/InlineSpinner.js @@ -28,7 +28,7 @@ export default createReactClass({ const imgClass = this.props.imgClassName || ""; let imageSource; - if (SettingsStore.isFeatureEnabled('feature_new_spinner')) { + if (SettingsStore.getValue('feature_new_spinner')) { imageSource = require("../../../../res/img/spinner.svg"); } else { imageSource = require("../../../../res/img/spinner.gif"); diff --git a/src/components/views/elements/ManageIntegsButton.js b/src/components/views/elements/ManageIntegsButton.js index ac8a98a94a..0990218c65 100644 --- a/src/components/views/elements/ManageIntegsButton.js +++ b/src/components/views/elements/ManageIntegsButton.js @@ -34,7 +34,7 @@ export default class ManageIntegsButton extends React.Component { if (!managers.hasManager()) { managers.openNoManagerDialog(); } else { - if (SettingsStore.isFeatureEnabled("feature_many_integration_managers")) { + if (SettingsStore.getValue("feature_many_integration_managers")) { managers.openAll(this.props.room); } else { managers.getPrimaryManager().open(this.props.room); diff --git a/src/components/views/elements/Spinner.js b/src/components/views/elements/Spinner.js index 033d4d13f4..4d2dcea90a 100644 --- a/src/components/views/elements/Spinner.js +++ b/src/components/views/elements/Spinner.js @@ -22,7 +22,7 @@ import SettingsStore from "../../../settings/SettingsStore"; const Spinner = ({w = 32, h = 32, imgClassName, message}) => { let imageSource; - if (SettingsStore.isFeatureEnabled('feature_new_spinner')) { + if (SettingsStore.getValue('feature_new_spinner')) { imageSource = require("../../../../res/img/spinner.svg"); } else { imageSource = require("../../../../res/img/spinner.gif"); diff --git a/src/components/views/messages/MessageEvent.js b/src/components/views/messages/MessageEvent.js index eb74270762..c3e5af2eb6 100644 --- a/src/components/views/messages/MessageEvent.js +++ b/src/components/views/messages/MessageEvent.js @@ -95,7 +95,7 @@ export default createReactClass({ } } - if (SettingsStore.isFeatureEnabled("feature_mjolnir")) { + if (SettingsStore.getValue("feature_mjolnir")) { const key = `mx_mjolnir_render_${this.props.mxEvent.getRoomId()}__${this.props.mxEvent.getId()}`; const allowRender = localStorage.getItem(key) === "true"; diff --git a/src/components/views/right_panel/UserInfo.js b/src/components/views/right_panel/UserInfo.js index b52792b3d1..71ee86a1ea 100644 --- a/src/components/views/right_panel/UserInfo.js +++ b/src/components/views/right_panel/UserInfo.js @@ -1428,7 +1428,7 @@ const UserInfoHeader = ({onClose, member, e2eStatus}) => { presenceLastActiveAgo = member.user.lastActiveAgo; presenceCurrentlyActive = member.user.currentlyActive; - if (SettingsStore.isFeatureEnabled("feature_custom_status")) { + if (SettingsStore.getValue("feature_custom_status")) { statusMessage = member.user._unstable_statusMessage; } } diff --git a/src/components/views/rooms/AppsDrawer.js b/src/components/views/rooms/AppsDrawer.js index 06dfffad30..8cf7a54da2 100644 --- a/src/components/views/rooms/AppsDrawer.js +++ b/src/components/views/rooms/AppsDrawer.js @@ -130,7 +130,7 @@ export default createReactClass({ }, _launchManageIntegrations: function() { - if (SettingsStore.isFeatureEnabled("feature_many_integration_managers")) { + if (SettingsStore.getValue("feature_many_integration_managers")) { IntegrationManagers.sharedInstance().openAll(); } else { IntegrationManagers.sharedInstance().getPrimaryManager().open(this.props.room, 'add_integ'); diff --git a/src/components/views/rooms/AuxPanel.js b/src/components/views/rooms/AuxPanel.js index ad66f0e151..521aeec406 100644 --- a/src/components/views/rooms/AuxPanel.js +++ b/src/components/views/rooms/AuxPanel.js @@ -104,7 +104,7 @@ export default createReactClass({ }, _rateLimitedUpdate: new RateLimitedFunc(function() { - if (SettingsStore.isFeatureEnabled("feature_state_counters")) { + if (SettingsStore.getValue("feature_state_counters")) { this.setState({counters: this._computeCounters()}); } }, 500), @@ -112,7 +112,7 @@ export default createReactClass({ _computeCounters: function() { let counters = []; - if (this.props.room && SettingsStore.isFeatureEnabled("feature_state_counters")) { + if (this.props.room && SettingsStore.getValue("feature_state_counters")) { const stateEvs = this.props.room.currentState.getStateEvents('re.jki.counter'); stateEvs.sort((a, b) => { return a.getStateKey() < b.getStateKey(); @@ -206,7 +206,7 @@ export default createReactClass({ />; let stateViews = null; - if (this.state.counters && SettingsStore.isFeatureEnabled("feature_state_counters")) { + if (this.state.counters && SettingsStore.getValue("feature_state_counters")) { let counters = []; this.state.counters.forEach((counter, idx) => { diff --git a/src/components/views/rooms/MemberTile.js b/src/components/views/rooms/MemberTile.js index 3be378b341..ebb8b7999d 100644 --- a/src/components/views/rooms/MemberTile.js +++ b/src/components/views/rooms/MemberTile.js @@ -50,7 +50,7 @@ export default createReactClass({ componentDidMount() { const cli = MatrixClientPeg.get(); - if (SettingsStore.isFeatureEnabled("feature_custom_status")) { + if (SettingsStore.getValue("feature_custom_status")) { const { user } = this.props.member; if (user) { user.on("User._unstable_statusMessage", this._onStatusMessageCommitted); @@ -209,7 +209,7 @@ export default createReactClass({ const presenceState = member.user ? member.user.presence : null; let statusMessage = null; - if (member.user && SettingsStore.isFeatureEnabled("feature_custom_status")) { + if (member.user && SettingsStore.getValue("feature_custom_status")) { statusMessage = this.state.statusMessage; } diff --git a/src/components/views/rooms/RoomHeader.js b/src/components/views/rooms/RoomHeader.js index 1dedd53d00..fe5b12c1c0 100644 --- a/src/components/views/rooms/RoomHeader.js +++ b/src/components/views/rooms/RoomHeader.js @@ -226,7 +226,7 @@ export default createReactClass({ title={_t("Settings")} />; } - if (this.props.onPinnedClick && SettingsStore.isFeatureEnabled('feature_pinning')) { + if (this.props.onPinnedClick && SettingsStore.getValue('feature_pinning')) { let pinsIndicator = null; if (this._hasUnreadPins()) { pinsIndicator = (
); diff --git a/src/components/views/rooms/Stickerpicker.js b/src/components/views/rooms/Stickerpicker.js index b48790a9cf..dba25a94cf 100644 --- a/src/components/views/rooms/Stickerpicker.js +++ b/src/components/views/rooms/Stickerpicker.js @@ -363,7 +363,7 @@ export default class Stickerpicker extends React.Component { */ _launchManageIntegrations() { // TODO: Open the right integration manager for the widget - if (SettingsStore.isFeatureEnabled("feature_many_integration_managers")) { + if (SettingsStore.getValue("feature_many_integration_managers")) { IntegrationManagers.sharedInstance().openAll( this.props.room, `type_${WidgetType.STICKERPICKER.preferred}`, diff --git a/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx b/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx index c646025bbe..c9ec4a6bc7 100644 --- a/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx @@ -237,7 +237,7 @@ export default class AppearanceUserSettingsTab extends React.Component; } } diff --git a/src/hooks/useSettings.ts b/src/hooks/useSettings.ts index 4d1e1d5bad..9534fccc4c 100644 --- a/src/hooks/useSettings.ts +++ b/src/hooks/useSettings.ts @@ -36,11 +36,11 @@ export const useSettingValue = (settingName: string, roomId: string = null, excl // Hook to fetch whether a feature is enabled and dynamically update when that changes export const useFeatureEnabled = (featureName: string, roomId: string = null) => { - const [enabled, setEnabled] = useState(SettingsStore.isFeatureEnabled(featureName, roomId)); + const [enabled, setEnabled] = useState(SettingsStore.getValue(featureName, roomId)); useEffect(() => { const ref = SettingsStore.watchSetting(featureName, roomId, () => { - setEnabled(SettingsStore.isFeatureEnabled(featureName, roomId)); + setEnabled(SettingsStore.getValue(featureName, roomId)); }); // clean-up return () => { diff --git a/src/rageshake/submit-rageshake.ts b/src/rageshake/submit-rageshake.ts index 76b0444052..80034691c8 100644 --- a/src/rageshake/submit-rageshake.ts +++ b/src/rageshake/submit-rageshake.ts @@ -143,7 +143,7 @@ export default async function sendBugReport(bugReportEndpoint: string, opts: IOp } // add labs options - const enabledLabs = SettingsStore.getLabsFeatures().filter(f => SettingsStore.isFeatureEnabled(f)); + const enabledLabs = SettingsStore.getLabsFeatures().filter(f => SettingsStore.getValue(f)); if (enabledLabs.length) { body.append('enabled_labs', enabledLabs.join(', ')); } diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index e64de8af16..6c1f9b8411 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -240,19 +240,6 @@ export default class SettingsStore { return _t(displayName as string); } - /** - * Returns a list of all available labs feature names - * @returns {string[]} The list of available feature names - */ - public static getLabsFeatures(): string[] { - const possibleFeatures = Object.keys(SETTINGS).filter((s) => SettingsStore.isFeature(s)); - - const enableLabs = SdkConfig.get()["enableLabs"]; - if (enableLabs) return possibleFeatures; - - return possibleFeatures.filter((s) => SettingsStore.getFeatureState(s) === "labs"); - } - /** * Determines if a setting is also a feature. * @param {string} settingName The setting to look up. @@ -263,39 +250,6 @@ export default class SettingsStore { return SETTINGS[settingName].isFeature; } - /** - * Determines if a given feature is enabled. The feature given must be a known - * feature. - * @param {string} settingName The name of the setting that is a feature. - * @param {String} roomId The optional room ID to validate in, may be null. - * @return {boolean} True if the feature is enabled, false otherwise - */ - public static isFeatureEnabled(settingName: string, roomId: string = null) { - if (!SettingsStore.isFeature(settingName)) { - throw new Error("Setting " + settingName + " is not a feature"); - } - - return SettingsStore.getValue(settingName, roomId); - } - - /** - * Sets a feature as enabled or disabled on the current device. - * @param {string} settingName The name of the setting. - * @param {boolean} value True to enable the feature, false otherwise. - * @returns {Promise} Resolves when the setting has been set. - */ - public static setFeatureEnabled(settingName: string, value: any): Promise { - // Verify that the setting is actually a setting - if (!SETTINGS[settingName]) { - throw new Error("Setting '" + settingName + "' does not appear to be a setting."); - } - if (!SettingsStore.isFeature(settingName)) { - throw new Error("Setting " + settingName + " is not a feature"); - } - - return SettingsStore.setValue(settingName, null, SettingLevel.DEVICE, value); - } - /** * Gets the value of a setting. The room ID is optional if the setting is not to * be applied to any particular room, otherwise it should be supplied. @@ -346,13 +300,6 @@ export default class SettingsStore { const minIndex = levelOrder.indexOf(level); if (minIndex === -1) throw new Error("Level " + level + " is not prioritized"); - if (SettingsStore.isFeature(settingName)) { - const configValue = SettingsStore.getFeatureState(settingName); - if (configValue === "enable") return true; - if (configValue === "disable") return false; - // else let it fall through the default process - } - const handlers = SettingsStore.getHandlers(settingName); // Check if we need to invert the setting at all. Do this after we get the setting @@ -611,24 +558,6 @@ export default class SettingsStore { return handlers; } - - private static getFeatureState(settingName: string): LabsFeatureState { - const featuresConfig = SdkConfig.get()['features']; - const enableLabs = SdkConfig.get()['enableLabs']; // we'll honour the old flag - - let featureState = enableLabs ? "labs" : "disable"; - if (featuresConfig && featuresConfig[settingName] !== undefined) { - featureState = featuresConfig[settingName]; - } - - const allowedStates = ['enable', 'disable', 'labs']; - if (!allowedStates.includes(featureState)) { - console.warn("Feature state '" + featureState + "' is invalid for " + settingName); - featureState = "disable"; // to prevent accidental features. - } - - return featureState; - } } // For debugging purposes diff --git a/src/stores/CustomRoomTagStore.js b/src/stores/CustomRoomTagStore.js index 1f24dc589a..39177181b4 100644 --- a/src/stores/CustomRoomTagStore.js +++ b/src/stores/CustomRoomTagStore.js @@ -137,7 +137,7 @@ class CustomRoomTagStore extends EventEmitter { } _getUpdatedTags() { - if (!SettingsStore.isFeatureEnabled("feature_custom_tags")) { + if (!SettingsStore.getValue("feature_custom_tags")) { return {}; // none } diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index f4c2d5050e..0f3138fe9e 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -136,7 +136,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { } private async readAndCacheSettingsFromStore() { - const tagsEnabled = SettingsStore.isFeatureEnabled("feature_custom_tags"); + const tagsEnabled = SettingsStore.getValue("feature_custom_tags"); await this.updateState({ tagsEnabled, }); diff --git a/src/stores/room-list/previews/ReactionEventPreview.ts b/src/stores/room-list/previews/ReactionEventPreview.ts index c8f2be9a6e..95cdc01c66 100644 --- a/src/stores/room-list/previews/ReactionEventPreview.ts +++ b/src/stores/room-list/previews/ReactionEventPreview.ts @@ -24,8 +24,8 @@ import DMRoomMap from "../../../utils/DMRoomMap"; export class ReactionEventPreview implements IPreview { public getTextFor(event: MatrixEvent, tagId?: TagID): string { - const showDms = SettingsStore.isFeatureEnabled("feature_roomlist_preview_reactions_dms"); - const showAll = SettingsStore.isFeatureEnabled("feature_roomlist_preview_reactions_all"); + const showDms = SettingsStore.getValue("feature_roomlist_preview_reactions_dms"); + const showAll = SettingsStore.getValue("feature_roomlist_preview_reactions_all"); if (!showAll && (!showDms || DMRoomMap.shared().getUserIdForRoomId(event.getRoomId()))) return null; From eda4e24926c00138db2fbbea850ce90b3875560b Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 17 Aug 2020 13:19:15 -0600 Subject: [PATCH 2/7] Update LabsUserSettings for new feature behaviour --- .../views/settings/tabs/user/LabsUserSettingsTab.js | 4 ++-- src/settings/SettingsStore.ts | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js index 2666671a30..6559448dfe 100644 --- a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js @@ -28,7 +28,7 @@ export class LabsSettingToggle extends React.Component { }; _onChange = async (checked) => { - await SettingsStore.setFeatureEnabled(this.props.featureId, checked); + await SettingsStore.setValue(this.props.featureId, null, SettingLevel.DEVICE, checked); this.forceUpdate(); }; @@ -46,7 +46,7 @@ export default class LabsUserSettingsTab extends React.Component { render() { const SettingsFlag = sdk.getComponent("views.elements.SettingsFlag"); - const flags = SettingsStore.getLabsFeatures().map(f => ); + const flags = SettingsStore.getFeatureSettingNames().map(f => ); return (
{_t("Labs")}
diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index 6c1f9b8411..ea7d965720 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -124,6 +124,14 @@ export default class SettingsStore { // Counter used for generation of watcher IDs private static watcherCount = 1; + /** + * Gets all the feature-style setting names. + * @returns {string[]} The names of the feature settings. + */ + public static getFeatureSettingNames(): string[] { + return Object.keys(SETTINGS).filter(n => SettingsStore.isFeature(n)); + } + /** * Watches for changes in a particular setting. This is done without any local echo * wrapping and fires whenever a change is detected in a setting's value, at any level. From 71643862c04b93976c3cc77b3d848e2d4fb4b30c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 17 Aug 2020 13:24:55 -0600 Subject: [PATCH 3/7] Implement new config style for features --- src/settings/SettingsStore.ts | 2 +- src/settings/handlers/ConfigSettingsHandler.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index ea7d965720..dd75f8d1c9 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -53,7 +53,7 @@ const LEVEL_HANDLERS = { [SettingLevel.ROOM_ACCOUNT]: new RoomAccountSettingsHandler(defaultWatchManager), [SettingLevel.ACCOUNT]: new AccountSettingsHandler(defaultWatchManager), [SettingLevel.ROOM]: new RoomSettingsHandler(defaultWatchManager), - [SettingLevel.CONFIG]: new ConfigSettingsHandler(), + [SettingLevel.CONFIG]: new ConfigSettingsHandler(featureNames), [SettingLevel.DEFAULT]: new DefaultSettingsHandler(defaultSettings, invertedDefaultSettings), }; diff --git a/src/settings/handlers/ConfigSettingsHandler.ts b/src/settings/handlers/ConfigSettingsHandler.ts index 3e8b1724c1..791c655a0d 100644 --- a/src/settings/handlers/ConfigSettingsHandler.ts +++ b/src/settings/handlers/ConfigSettingsHandler.ts @@ -24,9 +24,24 @@ import {isNullOrUndefined} from "matrix-js-sdk/src/utils"; * roomId parameter. */ export default class ConfigSettingsHandler extends SettingsHandler { + public constructor(private featureNames: string[]) { + super(); + } + public getValue(settingName: string, roomId: string): any { const config = SdkConfig.get() || {}; + if (this.featureNames.includes(settingName)) { + const labsConfig = config["features"] || {}; + const val = labsConfig[settingName]; + if (isNullOrUndefined(val)) return null; // no definition at this level + if (val === true || val === false) return val; // new style: mapped as a boolean + if (val === "enable") return true; // backwards compat + if (val === "disable") return false; // backwards compat + if (val === "labs") return null; // backwards compat, no override + return null; // fallback in the case of invalid input + } + // Special case themes if (settingName === "theme") { return config["default_theme"]; From 2c0e6c859ab21c0e23b5a5a97cc3d7225819ec58 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 17 Aug 2020 13:32:33 -0600 Subject: [PATCH 4/7] Update settings documentation for features --- docs/settings.md | 85 ++++++++++++------------------------------------ 1 file changed, 20 insertions(+), 65 deletions(-) diff --git a/docs/settings.md b/docs/settings.md index 46e4a68fdb..40c3e6a7d6 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -9,13 +9,13 @@ of dealing with the different levels and exposes easy to use getters and setters ## Levels Granular Settings rely on a series of known levels in order to use the correct value for the scenario. These levels, in -order of prioirty, are: +order of priority, are: * `device` - The current user's device * `room-device` - The current user's device, but only when in a specific room * `room-account` - The current user's account, but only when in a specific room * `account` - The current user's account * `room` - A specific room (setting for all members of the room) -* `config` - Values are defined by the `settingDefaults` key (usually) in `config.json` +* `config` - Values are defined by the `settingDefaults` key (usually) in `config.tson` * `default` - The hardcoded default for the settings Individual settings may control which levels are appropriate for them as part of the defaults. This is often to ensure @@ -25,33 +25,10 @@ that room administrators cannot force account-only settings upon participants. ## Settings Settings are the different options a user may set or experience in the application. These are pre-defined in -`src/settings/Settings.js` under the `SETTINGS` constant and have the following minimum requirements: -``` -// The ID is used to reference the setting throughout the application. This must be unique. -"theSettingId": { - // The levels this setting supports is required. In `src/settings/Settings.js` there are various pre-set arrays - // for this option - they should be used where possible to avoid copy/pasting arrays across settings. - supportedLevels: [...], +`src/settings/Settings.ts` under the `SETTINGS` constant, and match the `ISetting` interface as defined there. - // The default for this setting serves two purposes: It provides a value if the setting is not defined at other - // levels, and it serves to demonstrate the expected type to other developers. The value isn't enforced, but it - // should be respected throughout the code. The default may be any data type. - default: false, - - // The display name has two notations: string and object. The object notation allows for different translatable - // strings to be used for different levels, while the string notation represents the string for all levels. - - displayName: _td("Change something"), // effectively `displayName: { "default": _td("Change something") }` - displayName: { - "room": _td("Change something for participants of this room"), - - // Note: the default will be used if the level requested (such as `device`) does not have a string defined here. - "default": _td("Change something"), - } -} -``` - -Settings that support the config level can be set in the config file under the `settingDefaults` key (note that some settings, like the "theme" setting, are special cased in the config file): +Settings that support the config level can be set in the config file under the `settingDefaults` key (note that some +settings, like the "theme" setting, are special cased in the config file): ```json { ... @@ -119,38 +96,29 @@ for you. If a display name cannot be found, it will return `null`. ## Features -Occasionally some parts of the application may be undergoing testing and are not quite production ready. These are -commonly known to be behind a "labs flag". Features behind lab flags must go through the granular settings system, and -look and act very much normal settings. The exception is that they must supply `isFeature: true` as part of the setting -definition and should go through the helper functions on `SettingsStore`. +Feature flags are just like regular settings with some underlying semantics for how they are meant to be used. Usually +a feature flag is used when a portion of the application is under development or not ready for full release yet, such +as new functionality or experimental ideas. In these cases, the feature name *should* be named with the `feature_*` +convention and must be tagged with `isFeature: true` in the setting definition. By doing so, the feature will automatically +appear in the "labs" section of the user's settings. -Although features have levels and a default value, the calculation of those options is blocked by the feature's state. -A feature's state is determined from the `SdkConfig` and is a little complex. If `enableLabs` (a legacy flag) is `true` -then the feature's state is `labs`, if it is `false`, the state is `disable`. If `enableLabs` is not set then the state -is determined from the `features` config, such as in the following: +Features can be controlled at the config level using the following structure: ```json "features": { - "feature_lazyloading": "labs" + "feature_lazyloading": true } ``` -In this example, `feature_lazyloading` is in the `labs` state. It may also be in the `enable` or `disable` state with a -similar approach. If the state is invalid, the feature is in the `disable` state. A feature's levels are only calculated -if it is in the `labs` state, therefore the default only applies in that scenario. If the state is `enable`, the feature -is always-on. -Once a feature flag has served its purpose, it is generally recommended to remove it and the associated feature flag -checks. This would enable the feature implicitly as it is part of the application now. +When `true`, the user will see the feature as enabled. Similarly, when `false` the user will see the feature as disabled. +The user will only be able to change/see these states if `showLabsSettings: true` is in the config. ### Determining if a feature is enabled -A simple call to `SettingsStore.isFeatureEnabled` will tell you if the feature is enabled. This will perform all the -required calculations to determine if the feature is enabled based upon the configuration and user selection. +Call `SettingsStore.getValue()` as you would for any other setting. ### Enabling a feature -Features can only be enabled if the feature is in the `labs` state, otherwise this is a no-op. To find the current set -of features in the `labs` state, call `SettingsStore.getLabsFeatures`. To set the value, call -`SettingsStore.setFeatureEnabled`. +Call `SettingsStore.setValue("feature_name", null, SettingLevel.DEVICE, true)`. ## Setting controllers @@ -162,7 +130,7 @@ kept up to date with the setting where it is otherwise not possible. An example they can only be considered enabled if the platform supports notifications, and enabling notifications requires additional steps to actually enable notifications. -For more information, see `src/settings/controllers/SettingController.js`. +For more information, see `src/settings/controllers/SettingController.ts`. ## Local echo @@ -222,7 +190,7 @@ The `SettingsStore` uses the hardcoded `LEVEL_ORDER` constant to ensure that it The array is checked from left to right, simulating the behaviour of overriding values from the higher levels. Each level should be defined in this array, including `default`. -Handlers (`src/settings/handlers/SettingsHandler.js`) represent a single level and are responsible for getting and +Handlers (`src/settings/handlers/SettingsHandler.ts`) represent a single level and are responsible for getting and setting values at that level. Handlers also provide additional information to the `SettingsStore` such as if the level is supported or if the current user may set values at the level. The `SettingsStore` will use the handler to enforce checks and manipulate settings. Handlers are also responsible for dealing with migration patterns or legacy settings for @@ -230,7 +198,7 @@ their level (for example, a setting being renamed or using a different key from Handlers are provided to the `SettingsStore` via the `LEVEL_HANDLERS` constant. `SettingsStore` will optimize lookups by only considering handlers that are supported on the platform. -Local echo is achieved through `src/settings/handlers/LocalEchoWrapper.js` which acts as a wrapper around a given +Local echo is achieved through `src/settings/handlers/LocalEchoWrapper.ts` which acts as a wrapper around a given handler. This is automatically applied to all defined `LEVEL_HANDLERS` and proxies the calls to the wrapped handler where possible. The echo is achieved by a simple object cache stored within the class itself. The cache is invalidated immediately upon the proxied save call succeeding or failing. @@ -240,20 +208,7 @@ Controllers are notified of changes by the `SettingsStore`, and are given the op ### Features -Features automatically get considered as `disabled` if they are not listed in the `SdkConfig` or `enableLabs` is -false/not set. Features are always checked against the configuration before going through the level order as they have -the option of being forced-on or forced-off for the application. This is done by the `features` section and looks -something like this: - -``` -"features": { - "feature_groups": "enable", - "feature_pinning": "disable", // the default - "feature_presence": "labs" -} -``` - -If `enableLabs` is true in the configuration, the default for features becomes `"labs"`. +See above for feature reference. ### Watchers From 3659115921ade127e46bf9a5f5b5abdd1147dc0e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 17 Aug 2020 13:37:16 -0600 Subject: [PATCH 5/7] Followup on some SettingsStore removals --- src/components/views/dialogs/UserSettingsDialog.js | 2 +- src/rageshake/submit-rageshake.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/dialogs/UserSettingsDialog.js b/src/components/views/dialogs/UserSettingsDialog.js index 820de713c1..ffde03fe31 100644 --- a/src/components/views/dialogs/UserSettingsDialog.js +++ b/src/components/views/dialogs/UserSettingsDialog.js @@ -116,7 +116,7 @@ export default class UserSettingsDialog extends React.Component { "mx_UserSettingsDialog_securityIcon", , )); - if (SdkConfig.get()['showLabsSettings'] || SettingsStore.getLabsFeatures().length > 0) { + if (SdkConfig.get()['showLabsSettings']) { tabs.push(new Tab( USER_LABS_TAB, _td("Labs"), diff --git a/src/rageshake/submit-rageshake.ts b/src/rageshake/submit-rageshake.ts index 80034691c8..1dcf929e86 100644 --- a/src/rageshake/submit-rageshake.ts +++ b/src/rageshake/submit-rageshake.ts @@ -143,7 +143,7 @@ export default async function sendBugReport(bugReportEndpoint: string, opts: IOp } // add labs options - const enabledLabs = SettingsStore.getLabsFeatures().filter(f => SettingsStore.getValue(f)); + const enabledLabs = SettingsStore.getFeatureSettingNames().filter(f => SettingsStore.getValue(f)); if (enabledLabs.length) { body.append('enabled_labs', enabledLabs.join(', ')); } From 4f851542ac882243b52a383106dc64d588fbd0e8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 17 Aug 2020 13:51:41 -0600 Subject: [PATCH 6/7] Implement force state for features like in the proposal --- .../views/settings/tabs/user/LabsUserSettingsTab.js | 3 ++- src/settings/SettingsStore.ts | 7 ++++++- test/end-to-end-tests/riot/config-template/config.json | 5 +---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js index 6559448dfe..eba5c6586d 100644 --- a/src/components/views/settings/tabs/user/LabsUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/LabsUserSettingsTab.js @@ -35,7 +35,8 @@ export class LabsSettingToggle extends React.Component { render() { const label = SettingsStore.getDisplayName(this.props.featureId); const value = SettingsStore.getValue(this.props.featureId); - return ; + const canChange = SettingsStore.canSetValue(this.props.featureId, null, SettingLevel.DEVICE); + return ; } } diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index dd75f8d1c9..9e146ad799 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -23,7 +23,6 @@ import AccountSettingsHandler from "./handlers/AccountSettingsHandler"; import RoomSettingsHandler from "./handlers/RoomSettingsHandler"; import ConfigSettingsHandler from "./handlers/ConfigSettingsHandler"; import { _t } from '../languageHandler'; -import SdkConfig from "../SdkConfig"; import dis from '../dispatcher/dispatcher'; import { ISetting, SETTINGS } from "./Settings"; import LocalEchoWrapper from "./handlers/LocalEchoWrapper"; @@ -435,6 +434,12 @@ export default class SettingsStore { throw new Error("Setting '" + settingName + "' does not appear to be a setting."); } + // When features are specified in the config.json, we force them as enabled or disabled. + if (SettingsStore.isFeature(settingName)) { + const configVal = SettingsStore.getValueAt(SettingLevel.CONFIG, settingName, roomId, true, true); + if (configVal === true || configVal === false) return false; + } + const handler = SettingsStore.getHandler(settingName, level); if (!handler) return false; return handler.canSetValue(settingName, roomId); diff --git a/test/end-to-end-tests/riot/config-template/config.json b/test/end-to-end-tests/riot/config-template/config.json index d0d3a288e4..b647d0bec8 100644 --- a/test/end-to-end-tests/riot/config-template/config.json +++ b/test/end-to-end-tests/riot/config-template/config.json @@ -9,10 +9,7 @@ "integrations_ui_url": "https://scalar.vector.im/", "integrations_rest_url": "https://scalar.vector.im/api", "bug_report_endpoint_url": "https://riot.im/bugreports/submit", - "features": { - "feature_groups": "labs", - "feature_pinning": "labs" - }, + "showLabsSettings": true, "default_federate": true, "welcomePageUrl": "home.html", "default_theme": "light", From 0fef86cc743916ed0124e6f4ef9e25d5b98e1554 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Aug 2020 11:52:02 -0600 Subject: [PATCH 7/7] Fix find & replace error --- docs/settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/settings.md b/docs/settings.md index 40c3e6a7d6..4172c72c15 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -15,7 +15,7 @@ order of priority, are: * `room-account` - The current user's account, but only when in a specific room * `account` - The current user's account * `room` - A specific room (setting for all members of the room) -* `config` - Values are defined by the `settingDefaults` key (usually) in `config.tson` +* `config` - Values are defined by the `settingDefaults` key (usually) in `config.json` * `default` - The hardcoded default for the settings Individual settings may control which levels are appropriate for them as part of the defaults. This is often to ensure