Merge pull request #1684 from matrix-org/luke/fix-tag-panel-shift-ctrl-click

Fix shift and shift-ctrl click in TagPanel

src/Keyboard.js

@@ -68,3 +68,12 @@ export function isOnlyCtrlOrCmdKeyEvent(ev) {
         return ev.ctrlKey && !ev.altKey && !ev.metaKey && !ev.shiftKey;
     }
 }
+
+export function isOnlyCtrlOrCmdIgnoreShiftKeyEvent(ev) {
+    const isMac = navigator.platform.toUpperCase().indexOf('MAC') >= 0;
+    if (isMac) {
+        return ev.metaKey && !ev.altKey && !ev.ctrlKey;
+    } else {
+        return ev.ctrlKey && !ev.altKey && !ev.metaKey;
+    }
+}

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

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,
         });
     },

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(() => {
+        this._tagStoreToken = TagOrderStore.addListener(() => {
-            FilterStore.getSelectedTags().forEach((tag) => {
+            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) {
+        if (this._tagStoreToken) {
-            this._filterStoreToken.remove();
+            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;
                 }

src/stores/FilterStore.js

@@ -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;

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) {