Fixes #12840, #12836, #13231, #13232

Additionally added tests for #10302 (#10284 (comment)), #10284 (#10217 (comment)), #11729
yiisoft · Dec 18, 2016 · 8d813f7 · 8d813f7
1 parent 24dae19
commit 8d813f7
Show file tree

Hide file tree

Showing 4 changed files with 1,009 additions and 27 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -21,7 +21,7 @@ Yii Framework 2 Change Log
 - Bug #12822: Fixed `yii\i18n\Formatter::asTimestamp()` to process timestamp with miliseconds correctly (h311ion)
 - Bug #12824: Enabled usage of `yii\mutex\FileMutex` on Windows systems (davidsonalencar)
 - Bug #12828: Fixed handling of nested arrays, objects in `\yii\grid\GridView::guessColumns` (githubjeka)
-- Bug #12836: Fixed `yii\widgets\GridView::filterUrl` to not ignore `#` part of filter URL (cebe)
+- Bug #12836: Fixed `yii\widgets\GridView::filterUrl` to not ignore `#` part of filter URL (cebe, arogachev)
 - Bug #12856: Fixed `yii\web\XmlResponseFormatter` to use `true` and `false` to represent booleans (samdark)
 - Bug #12879: Console progress bar was not working properly in Windows terminals (samdark, kids-return)
 - Bug #12880: Fixed `yii\behaviors\AttributeTypecastBehavior` marks attributes with `null` value as 'dirty' (klimov-paul)
@@ -36,6 +36,8 @@ Yii Framework 2 Change Log
 - Bug #13159: Fixed `destroy` method in `yii.captcha.js` which did not work as expected (arogachev)
 - Bug #13198: Fixed order of checks in `yii\validators\IpValidator` that sometimes caused wrong error message (silverfire)
 - Bug #13200: Creating Urls for routes specified in `yii\rest\UrlRule::$extraPatterns` did not work if no HTTP verb was specified (cebe)
+- Bug #13231: Fixed `destroy` method in `yii.gridView.js` which did not work as expected (arogachev)
+- Bug #13232: Event handlers were not detached with changed selector in `yii.gridView.js` (arogachev)
 - Bug #13108: Fix execute command with negative integer parameter (pana1990, uaoleg)
 - Enh #475: Added Bash and Zsh completion support for the `./yii` command (cebe, silverfire)
 - Enh #6242: Access to validator in inline validation (arogachev)

diff --git a/framework/assets/yii.gridView.js b/framework/assets/yii.gridView.js
@@ -16,7 +16,7 @@
         } else if (typeof method === 'object' || !method) {
             return methods.init.apply(this, arguments);
         } else {
-            $.error('Method ' + method + ' does not exist on jQuery.yiiGridView');
+            $.error('Method ' + method + ' does not exist in jQuery.yiiGridView');
             return false;
         }
     };
@@ -50,6 +50,32 @@
         afterFilter: 'afterFilter'
     };
 
+    /**
+     * Used for storing active event handlers and removing them later.
+     * The structure of single event handler is:
+     *
+     * {
+     *     gridViewId: {
+     *         type: {
+     *             event: '...',
+     *             selector: '...'
+     *         }
+     *     }
+     * }
+     *
+     * Used types:
+     *
+     * - filter, used for filtering grid with elements found by filterSelector
+     * - checkRow, used for checking single row
+     * - checkAllRows, used for checking all rows with according "Check all" checkbox
+     *
+     * event is the name of event, for example: 'change.yiiGridView'
+     * selector is a jQuery selector for finding elements
+     *
+     * @type {{}}
+     */
+    var gridEventHandlers = {};
+
     var methods = {
         init: function (options) {
             return this.each(function () {
@@ -62,32 +88,32 @@
 
                 gridData[id] = $.extend(gridData[id], {settings: settings});
 
+                var filterEvents = 'change.yiiGridView keydown.yiiGridView';
                 var enterPressed = false;
-                $(document).off('change.yiiGridView keydown.yiiGridView', settings.filterSelector)
-                    .on('change.yiiGridView keydown.yiiGridView', settings.filterSelector, function (event) {
-                        if (event.type === 'keydown') {
-                            if (event.keyCode !== 13) {
-                                return; // only react to enter key
-                            } else {
-                                enterPressed = true;
-                            }
+                initEventHandler($e, 'filter', filterEvents, settings.filterSelector, function (event) {
+                    if (event.type === 'keydown') {
+                        if (event.keyCode !== 13) {
+                            return; // only react to enter key
                         } else {
-                            // prevent processing for both keydown and change events
-                            if (enterPressed) {
-                                enterPressed = false;
-                                return;
-                            }
+                            enterPressed = true;
+                        }
+                    } else {
+                        // prevent processing for both keydown and change events
+                        if (enterPressed) {
+                            enterPressed = false;
+                            return;
                         }
+                    }
 
-                        methods.applyFilter.apply($e);
+                    methods.applyFilter.apply($e);
 
-                        return false;
-                    });
+                    return false;
+                });
             });
         },
 
         applyFilter: function () {
-            var $grid = $(this), event;
+            var $grid = $(this);
             var settings = gridData[$grid.attr('id')].settings;
             var data = {};
             $.each($(settings.filterSelector).serializeArray(), function () {
@@ -119,8 +145,8 @@
             var pos = settings.filterUrl.indexOf('?');
             var url = pos < 0 ? settings.filterUrl : settings.filterUrl.substring(0, pos);
             var hashPos = settings.filterUrl.indexOf('#');
-            if (hashPos >= 0) {
-                url += settings.filterUrl.substring(pos);
+            if (pos >= 0 && hashPos >= 0) {
+                url += settings.filterUrl.substring(hashPos);
             }
 
             $grid.find('form.gridview-filter-form').remove();
@@ -137,7 +163,7 @@
                 });
             });
 
-            event = $.Event(gridEvents.beforeFilter);
+            var event = $.Event(gridEvents.beforeFilter);
             $grid.trigger(event);
             if (event.result === false) {
                 return;
@@ -161,10 +187,10 @@
             var checkAll = "#" + id + " input[name='" + options.checkAll + "']";
             var inputs = options['class'] ? "input." + options['class'] : "input[name='" + options.name + "']";
             var inputsEnabled = "#" + id + " " + inputs + ":enabled";
-            $(document).off('click.yiiGridView', checkAll).on('click.yiiGridView', checkAll, function () {
+            initEventHandler($grid, 'checkAllRows', 'click.yiiGridView', checkAll, function () {
                 $grid.find(inputs + ":enabled").prop('checked', this.checked);
             });
-            $(document).off('click.yiiGridView', inputsEnabled).on('click.yiiGridView', inputsEnabled, function () {
+            initEventHandler($grid, 'checkRow', 'click.yiiGridView', inputsEnabled, function () {
                 var all = $grid.find(inputs).length == $grid.find(inputs + ":checked").length;
                 $grid.find("input[name='" + options.checkAll + "']").prop('checked', all);
             });
@@ -183,15 +209,45 @@
         },
 
         destroy: function () {
-            return this.each(function () {
-                $(window).unbind('.yiiGridView');
-                $(this).removeData('yiiGridView');
+            var events = ['.yiiGridView', gridEvents.beforeFilter, gridEvents.afterFilter].join(' ');
+            this.off(events);
+
+            var id = $(this).attr('id');
+            $.each(gridEventHandlers[id], function (type, data) {
+                $(document).off(data.event, data.selector);
             });
+
+            delete gridData[id];
+
+            return this;
         },
 
         data: function () {
             var id = $(this).attr('id');
             return gridData[id];
         }
     };
+
+    /**
+     * Used for attaching event handler and prevent of duplicating them. With each call previously attached handler of
+     * the same type is removed even selector was changed.
+     * @param {jQuery} $gridView According jQuery grid view element
+     * @param {string} type Type of the event which acts like a key
+     * @param {string} event Event name, for example 'change.yiiGridView'
+     * @param {string} selector jQuery selector
+     * @param {function} callback The actual function to be executed with this event
+     */
+    function initEventHandler($gridView, type, event, selector, callback) {
+        var id = $gridView.attr('id');
+        var prevHandler = gridEventHandlers[id];
+        if (prevHandler !== undefined && prevHandler[type] !== undefined) {
+            var data = prevHandler[type];
+            $(document).off(data.event, data.selector);
+        }
+        if (prevHandler === undefined) {
+            gridEventHandlers[id] = {};
+        }
+        $(document).on(event, selector, callback);
+        gridEventHandlers[id][type] = {event: event, selector: selector};
+    }
 })(window.jQuery);
diff --git a/tests/js/data/yii.gridView.html b/tests/js/data/yii.gridView.html
@@ -0,0 +1,170 @@
+<!-- Filters for testing of multiple grid views -->
+
+<div id="w-common-filters">
+  <input name="PostSearch[id]" type="text">
+  <input name="PostSearch[name]" type="text">
+</div>
+
+<!-- The main setup -->
+
+<div id="w0" class="grid-view">
+  <table>
+    <thead>
+      <tr>
+        <th><input id="w0-check-all" name="selection_all" value="1" type="checkbox"></th>
+        <th>Name</th>
+        <th>Category</th>
+        <th>Tags</th>
+      </tr>
+      <tr id="w0-filters">
+        <td>&nbsp;</td>
+        <td><input id="w0-name" name="PostSearch[name]" type="text"></td>
+        <td>
+          <select id="w0-category" name="PostSearch[category_id]">
+            <option value="" selected>None</option>
+            <option value="1">Programming</option>
+            <option value="2">Traveling</option>
+          </select>
+        </td>
+        <td>
+          <select id="w0-tags" name="PostSearch[tags][]" multiple>
+            <option value="1">html</option>
+            <option value="2">css</option>
+            <option value="3">js</option>
+            <option value="4">php</option>
+          </select>
+        </td>
+      </tr>
+    </thead>
+    <tbody>
+      <tr data-key="1">
+        <td><input class="w0-check-row" name="selection[]" value="1" type="checkbox"></td>
+        <td>Name 1</td>
+        <td>Programming</td>
+        <td>html, css</td>
+      </tr>
+      <tr data-key="2">
+        <td><input class="w0-check-row" name="selection[]" value="2" type="checkbox"></td>
+        <td>Name 2</td>
+        <td>Programming</td>
+        <td>js</td>
+      </tr>
+      <tr data-key="3">
+        <td><input class="w0-check-row" name="selection[]" value="3" type="checkbox"></td>
+        <td>Name 3</td>
+        <td>Programming</td>
+        <td>php</td>
+      </tr>
+      </tbody>
+  </table>
+</div>
+
+<!-- The basic setup, used for testing of multiple grid views -->
+
+<div id="w1" class="grid-view">
+  <table>
+    <thead>
+      <tr>
+        <th><input name="selection_all" value="1" type="checkbox"></th>
+        <th>ID</th>
+        <th>Name</th>
+      </tr>
+      <tr id="w1-filters">
+        <td>&nbsp;</td>
+        <td><input name="PostSearch[id]" type="text"></td>
+        <td><input name="PostSearch[name]" type="text"></td>
+      </tr>
+    </thead>
+    <tbody>
+      <tr data-key="1">
+        <td><input name="selection[]" value="1" type="checkbox"></td>
+        <td>1</td>
+        <td>Name 1</td>
+      </tr>
+      <tr data-key="2">
+        <td><input name="selection[]" value="2" type="checkbox"></td>
+        <td>2</td>
+        <td>Name 2</td>
+      </tr>
+    </tbody>
+  </table>
+</div>
+
+<!-- https://github.com/yiisoft/yii2/pull/10284 -->
+
+<div id="w2">
+  <table>
+    <thead>
+      <tr>
+        <th>Name</th>
+        <th>Tags</th>
+      </tr>
+      <tr id="w2-filters">
+        <td><input name="PostSearch[name]" type="text"></td>
+        <td>
+          <input type="hidden" name="PostSearch[tags]" value="-1">
+          <select id="w2-tags" name="PostSearch[tags][]" multiple>
+            <option value="1">html</option>
+            <option value="2">css</option>
+            <option value="3">js</option>
+            <option value="4">php</option>
+          </select>
+        </td>
+      </tr>
+    </thead>
+    <tbody>
+      <tr data-key="1">
+        <td>Name 1</td>
+        <td>html, css</td>
+      </tr>
+      <tr data-key="2">
+        <td>Name 2</td>
+        <td>js</td>
+      </tr>
+      <tr data-key="3">
+        <td>Name 3</td>
+        <td>php</td>
+      </tr>
+    </tbody>
+  </table>
+</div>
+
+<!-- Setup for testing that event handlers are correctly removed with new selectors -->
+
+<div id="w3">
+  <table>
+    <thead>
+      <tr>
+        <th>
+          <input name="selection_all" value="1" type="checkbox">
+          <input name="selection_all2" value="1" type="checkbox">
+        </th>
+        <th>ID</th>
+        <th>Name</th>
+      </tr>
+      <tr id="w3-filters">
+        <td>&nbsp;</td>
+        <td><input name="PostSearch[id]" type="text"></td>
+        <td><input name="PostSearch[name]" type="text"></td>
+      </tr>
+    </thead>
+    <tbody>
+      <tr data-key="1">
+        <td>
+          <input class="w3-check-row" name="selection[]" value="1" type="checkbox">
+          <input name="selection2[]" value="1" type="checkbox">
+        </td>
+        <td>1</td>
+        <td>Name 1</td>
+      </tr>
+      <tr data-key="2">
+        <td>
+          <input class="w3-check-row" name="selection[]" value="2" type="checkbox">
+          <input name="selection2[]" value="2" type="checkbox">
+        </td>
+        <td>2</td>
+        <td>Name 2</td>
+      </tr>
+    </tbody>
+  </table>
+</div>