Browse Source

Fix EscapeActions click in handling

Fixes a bug introduced in 07cc454 and one introduced in 22e854c.
Maxime Quandalle 9 years ago
parent
commit
60712e1ac4
3 changed files with 33 additions and 21 deletions
  1. 1 1
      client/components/cards/details.js
  2. 8 3
      client/config/router.js
  3. 24 17
      client/lib/escapeActions.js

+ 1 - 1
client/components/cards/details.js

@@ -118,6 +118,6 @@ Template.cardMorePopup.events({
 EscapeActions.register('detailsPane',
   function() { Utils.goBoardId(Session.get('currentBoard')); },
   function() { return ! Session.equals('currentCard', null); }, {
-    noClickEscapeOn: '.js-card-details'
+    noClickEscapeOn: '.js-card-details,.board-sidebar,#header'
   }
 );

+ 8 - 3
client/config/router.js

@@ -14,9 +14,14 @@ FlowRouter.route('/', {
 FlowRouter.route('/b/:id/:slug', {
   name: 'board',
   action: function(params) {
-    EscapeActions.executeAll();
-
-    Session.set('currentBoard', params.id);
+    let currentBoard = params.id;
+    // If we close a card, we'll execute again this route action but we don't
+    // want to excape every current actions (filters, etc.)
+    if (Session.get('currentBoard') !== currentBoard) {
+      EscapeActions.executeAll();
+    }
+
+    Session.set('currentBoard', currentBoard);
     Session.set('currentCard', null);
 
     BlazeLayout.render('defaultLayout', { content: 'board' });

+ 24 - 17
client/lib/escapeActions.js

@@ -18,19 +18,25 @@ EscapeActions = {
   ],
 
   register: function(label, action, condition = () => true, options = {}) {
-    var priority = this.hierarchy.indexOf(label);
+    const priority = this.hierarchy.indexOf(label);
     if (priority === -1) {
       throw Error('You must define the label in the EscapeActions hierarchy');
     }
 
-    this._actions.push({
+    let enabledOnClick = options.enabledOnClick;
+    if (_.isUndefined(enabledOnClick)) {
+      enabledOnClick = true;
+    }
+
+    let noClickEscapeOn = options.noClickEscapeOn;
+
+    this._actions[priority] = {
       priority,
       condition,
       action,
-      noClickEscapeOn: options.noClickEscapeOn,
-      enabledOnClick: !! options.enabledOnClick
-    });
-    this._actions = _.sortBy(this._actions, (a) => { return a.priority; });
+      noClickEscapeOn,
+      enabledOnClick
+    };
   },
 
   executeLowest: function() {
@@ -69,27 +75,28 @@ EscapeActions = {
   },
 
   _execute: function(options) {
-    var maxLabel = options.maxLabel;
-    var multipleActions = options.multipleActions;
-    var isClick = !! options.isClick;
-    var clickTarget = options.clickTarget;
+    const maxLabel = options.maxLabel;
+    const multipleActions = options.multipleActions;
+    const isClick = !! options.isClick;
+    const clickTarget = options.clickTarget;
+
+    let executedAtLeastOne = false;
+    let maxPriority;
 
-    var maxPriority, currentAction;
-    var executedAtLeastOne = false;
     if (! maxLabel)
       maxPriority = Infinity;
     else
       maxPriority = this.hierarchy.indexOf(maxLabel);
 
-    for (var i = 0; i < this._actions.length; i++) {
-      currentAction = this._actions[i];
+    for (let i = 0; i < this._actions.length; i++) {
+      let currentAction = this._actions[i];
       if (currentAction.priority > maxPriority)
         return executedAtLeastOne;
 
       if (isClick && this._stopClick(currentAction, clickTarget))
         return executedAtLeastOne;
 
-      var isEnabled = currentAction.enabledOnClick || ! isClick;
+      let isEnabled = currentAction.enabledOnClick || ! isClick;
       if (isEnabled && currentAction.condition()) {
         currentAction.action();
         executedAtLeastOne = true;
@@ -150,8 +157,8 @@ Mousetrap.bindGlobal('esc', function() {
 // On a left click on the document, we try to exectute one escape action (eg,
 // close the popup). We don't execute any action if the user has clicked on a
 // link or a button.
-$(document.body).on('click', function(evt) {
-  if (evt.which === 1 &&
+$(document).on('click', function(evt) {
+  if (evt.button === 0 &&
     $(evt.target).closest('a,button,.is-editable').length === 0) {
     EscapeActions.clickExecute(evt.target, 'multiselection');
   }