From 85cdd888e81c2dbe0217970f65691a188d4e22df Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 3 Jan 2018 11:33:59 +0000 Subject: [PATCH 1/3] Combine TagOrderStore and FilterStore so that shift-click semantics can work. The store that computes the shift-click rules has to be aware of the actual order of tags displayed, so they must be done in the same store. --- src/components/structures/TagPanel.js | 10 +-- src/components/views/rooms/RoomList.js | 16 ++-- src/stores/FilterStore.js | 115 ------------------------- src/stores/TagOrderStore.js | 63 ++++++++++++++ 4 files changed, 72 insertions(+), 132 deletions(-) delete mode 100644 src/stores/FilterStore.js diff --git a/src/components/structures/TagPanel.js b/src/components/structures/TagPanel.js index aa35101197..531c247ea6 100644 --- a/src/components/structures/TagPanel.js +++ b/src/components/structures/TagPanel.js @@ -17,7 +17,6 @@ limitations under the License. import React from 'react'; import PropTypes from 'prop-types'; import { MatrixClient } from 'matrix-js-sdk'; -import FilterStore from '../../stores/FilterStore'; import TagOrderStore from '../../stores/TagOrderStore'; import GroupActions from '../../actions/GroupActions'; @@ -44,20 +43,13 @@ const TagPanel = React.createClass({ this.unmounted = false; this.context.matrixClient.on("Group.myMembership", this._onGroupMyMembership); - this._filterStoreToken = FilterStore.addListener(() => { - if (this.unmounted) { - return; - } - this.setState({ - selectedTags: FilterStore.getSelectedTags(), - }); - }); this._tagOrderStoreToken = TagOrderStore.addListener(() => { if (this.unmounted) { return; } this.setState({ orderedTags: TagOrderStore.getOrderedTags() || [], + selectedTags: TagOrderStore.getSelectedTags(), }); }); // This could be done by anything with a matrix client diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index f14ba5529d..82516cc218 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -28,7 +28,7 @@ const rate_limited_func = require('../../../ratelimitedfunc'); const Rooms = require('../../../Rooms'); import DMRoomMap from '../../../utils/DMRoomMap'; const Receipt = require('../../../utils/Receipt'); -import FilterStore from '../../../stores/FilterStore'; +import TagOrderStore from '../../../stores/TagOrderStore'; import GroupStoreCache from '../../../stores/GroupStoreCache'; const HIDE_CONFERENCE_CHANS = true; @@ -95,8 +95,8 @@ module.exports = React.createClass({ // All rooms that should be kept in the room list when filtering this._visibleRooms = []; // When the selected tags are changed, initialise a group store if necessary - this._filterStoreToken = FilterStore.addListener(() => { - FilterStore.getSelectedTags().forEach((tag) => { + this._tagStoreToken = TagOrderStore.addListener(() => { + TagOrderStore.getSelectedTags().forEach((tag) => { if (tag[0] !== '+' || this._groupStores[tag]) { return; } @@ -182,8 +182,8 @@ module.exports = React.createClass({ MatrixClientPeg.get().removeListener("Group.myMembership", this._onGroupMyMembership); } - if (this._filterStoreToken) { - this._filterStoreToken.remove(); + if (this._tagStoreToken) { + this._tagStoreToken.remove(); } if (this._groupStoreTokens.length > 0) { @@ -298,14 +298,14 @@ module.exports = React.createClass({ // Update which rooms and users should appear according to which tags are selected updateVisibleRooms: function() { this._visibleRooms = []; - FilterStore.getSelectedTags().forEach((tag) => { + TagOrderStore.getSelectedTags().forEach((tag) => { (this._visibleRoomsForGroup[tag] || []).forEach( (roomId) => this._visibleRooms.push(roomId), ); }); this.setState({ - selectedTags: FilterStore.getSelectedTags(), + selectedTags: TagOrderStore.getSelectedTags(), }, () => { this.refreshRoomList(); }); @@ -362,7 +362,7 @@ module.exports = React.createClass({ // Used to split rooms via tags const tagNames = Object.keys(room.tags); - // Apply TagPanel filtering, derived from FilterStore + // Apply TagPanel filtering, derived from TagOrderStore if (!this.isRoomInSelectedTags(room)) { return; } diff --git a/src/stores/FilterStore.js b/src/stores/FilterStore.js deleted file mode 100644 index 8078a13ffd..0000000000 --- a/src/stores/FilterStore.js +++ /dev/null @@ -1,115 +0,0 @@ -/* -Copyright 2017 Vector Creations Ltd - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ -import {Store} from 'flux/utils'; -import dis from '../dispatcher'; -import Analytics from '../Analytics'; - -const INITIAL_STATE = { - allTags: [], - selectedTags: [], - // Last selected tag when shift was not being pressed - anchorTag: null, -}; - -/** - * A class for storing application state for filtering via TagPanel. - */ -class FilterStore extends Store { - constructor() { - super(dis); - - // Initialise state - this._state = INITIAL_STATE; - } - - _setState(newState) { - this._state = Object.assign(this._state, newState); - this.__emitChange(); - } - - __onDispatch(payload) { - switch (payload.action) { - case 'all_tags' : - this._setState({ - allTags: payload.tags, - }); - break; - case 'select_tag': { - let newTags = []; - // Shift-click semantics - if (payload.shiftKey) { - // Select range of tags - let start = this._state.allTags.indexOf(this._state.anchorTag); - let end = this._state.allTags.indexOf(payload.tag); - - if (start === -1) { - start = end; - } - if (start > end) { - const temp = start; - start = end; - end = temp; - } - newTags = payload.ctrlOrCmdKey ? this._state.selectedTags : []; - newTags = [...new Set( - this._state.allTags.slice(start, end + 1).concat(newTags), - )]; - } else { - if (payload.ctrlOrCmdKey) { - // Toggle individual tag - if (this._state.selectedTags.includes(payload.tag)) { - newTags = this._state.selectedTags.filter((t) => t !== payload.tag); - } else { - newTags = [...this._state.selectedTags, payload.tag]; - } - } else { - // Select individual tag - newTags = [payload.tag]; - } - // Only set the anchor tag if the tag was previously unselected, otherwise - // the next range starts with an unselected tag. - if (!this._state.selectedTags.includes(payload.tag)) { - this._setState({ - anchorTag: payload.tag, - }); - } - } - - this._setState({ - selectedTags: newTags, - }); - - Analytics.trackEvent('FilterStore', 'select_tag'); - } - break; - case 'deselect_tags': - this._setState({ - selectedTags: [], - }); - Analytics.trackEvent('FilterStore', 'deselect_tags'); - break; - } - } - - getSelectedTags() { - return this._state.selectedTags; - } -} - -if (global.singletonFilterStore === undefined) { - global.singletonFilterStore = new FilterStore(); -} -export default global.singletonFilterStore; diff --git a/src/stores/TagOrderStore.js b/src/stores/TagOrderStore.js index 633ffc7e9c..9c9427284e 100644 --- a/src/stores/TagOrderStore.js +++ b/src/stores/TagOrderStore.js @@ -15,12 +15,17 @@ limitations under the License. */ import {Store} from 'flux/utils'; import dis from '../dispatcher'; +import Analytics from '../Analytics'; const INITIAL_STATE = { orderedTags: null, orderedTagsAccountData: null, hasSynced: false, joinedGroupIds: null, + + selectedTags: [], + // Last selected tag when shift was not being pressed + anchorTag: null, }; /** @@ -93,6 +98,60 @@ class TagOrderStore extends Store { this._setState({orderedTags}); break; } + case 'select_tag': { + let newTags = []; + // Shift-click semantics + if (payload.shiftKey) { + // Select range of tags + let start = this._state.orderedTags.indexOf(this._state.anchorTag); + let end = this._state.orderedTags.indexOf(payload.tag); + + if (start === -1) { + start = end; + } + if (start > end) { + const temp = start; + start = end; + end = temp; + } + newTags = payload.ctrlOrCmdKey ? this._state.selectedTags : []; + newTags = [...new Set( + this._state.orderedTags.slice(start, end + 1).concat(newTags), + )]; + } else { + if (payload.ctrlOrCmdKey) { + // Toggle individual tag + if (this._state.selectedTags.includes(payload.tag)) { + newTags = this._state.selectedTags.filter((t) => t !== payload.tag); + } else { + newTags = [...this._state.selectedTags, payload.tag]; + } + } else { + // Select individual tag + newTags = [payload.tag]; + } + // Only set the anchor tag if the tag was previously unselected, otherwise + // the next range starts with an unselected tag. + if (!this._state.selectedTags.includes(payload.tag)) { + this._setState({ + anchorTag: payload.tag, + }); + } + } + + this._setState({ + selectedTags: newTags, + }); + + Analytics.trackEvent('FilterStore', 'select_tag'); + } + break; + case 'deselect_tags': + this._setState({ + selectedTags: [], + }); + Analytics.trackEvent('FilterStore', 'deselect_tags'); + break; case 'on_logged_out': { // Reset state without pushing an update to the view, which generally assumes that // the matrix client isn't `null` and so causing a re-render will cause NPEs. @@ -129,6 +188,10 @@ class TagOrderStore extends Store { getOrderedTags() { return this._state.orderedTags; } + + getSelectedTags() { + return this._state.selectedTags; + } } if (global.singletonTagOrderStore === undefined) { From d49551998659b9287adf07afb49f73667eb776b7 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 3 Jan 2018 11:39:15 +0000 Subject: [PATCH 2/3] Fix shift-ctrl-click isOnlyCtrlOrCmdKeyEvent is predicated on !shiftKey, so another function was needed for cases where we ignore other keys --- src/Keyboard.js | 9 +++++++++ src/components/views/elements/TagTile.js | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Keyboard.js b/src/Keyboard.js index 9c872e1c66..8e6983fae5 100644 --- a/src/Keyboard.js +++ b/src/Keyboard.js @@ -68,3 +68,12 @@ export function isOnlyCtrlOrCmdKeyEvent(ev) { return ev.ctrlKey && !ev.altKey && !ev.metaKey && !ev.shiftKey; } } + +export function isCtrlOrCmdKeyEvent(ev) { + const isMac = navigator.platform.toUpperCase().indexOf('MAC') >= 0; + if (isMac) { + return ev.metaKey && !ev.ctrlKey; + } else { + return ev.ctrlKey && !ev.metaKey; + } +} diff --git a/src/components/views/elements/TagTile.js b/src/components/views/elements/TagTile.js index eb9a5cbdd9..2457bb1760 100644 --- a/src/components/views/elements/TagTile.js +++ b/src/components/views/elements/TagTile.js @@ -20,7 +20,7 @@ import classNames from 'classnames'; import { MatrixClient } from 'matrix-js-sdk'; import sdk from '../../../index'; import dis from '../../../dispatcher'; -import { isOnlyCtrlOrCmdKeyEvent } from '../../../Keyboard'; +import { isCtrlOrCmdKeyEvent } from '../../../Keyboard'; import FlairStore from '../../../stores/FlairStore'; @@ -76,7 +76,7 @@ export default React.createClass({ dis.dispatch({ action: 'select_tag', tag: this.props.tag, - ctrlOrCmdKey: isOnlyCtrlOrCmdKeyEvent(e), + ctrlOrCmdKey: isCtrlOrCmdKeyEvent(e), shiftKey: e.shiftKey, }); }, From d5e2a73d99df1ac74d319294edea335b684ae967 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Thu, 4 Jan 2018 12:06:13 +0000 Subject: [PATCH 3/3] Add alt condition back in, rename to specify igorance of shift because this is really what this function is for - we want to ignore specifically shift but not necessarily alt (despite this probably not having any real impact). --- src/Keyboard.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Keyboard.js b/src/Keyboard.js index 8e6983fae5..bf83a1a05f 100644 --- a/src/Keyboard.js +++ b/src/Keyboard.js @@ -69,11 +69,11 @@ export function isOnlyCtrlOrCmdKeyEvent(ev) { } } -export function isCtrlOrCmdKeyEvent(ev) { +export function isOnlyCtrlOrCmdIgnoreShiftKeyEvent(ev) { const isMac = navigator.platform.toUpperCase().indexOf('MAC') >= 0; if (isMac) { - return ev.metaKey && !ev.ctrlKey; + return ev.metaKey && !ev.altKey && !ev.ctrlKey; } else { - return ev.ctrlKey && !ev.metaKey; + return ev.ctrlKey && !ev.altKey && !ev.metaKey; } }