From 3894b16a6bb022be9b1836de4c05dd63b806df3b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 6 Nov 2021 14:01:21 -0700 Subject: [PATCH 1/9] Make enter key in list view filter select the next item --- src/javascripts/Directives/listView.js | 15 ++++++++++++--- src/javascripts/Factories/hotkeyFactory.js | 15 ++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 5cdba57d..308680f1 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -24,6 +24,7 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk // helper variables let prevIndex = -1, + firstFilteredIndex = -1, eventListeners = { click: e => $scope.$apply(() => $scope.onParentClick(e)), keydown: e => $scope.$apply(() => { @@ -220,15 +221,23 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk return true; }; - $scope.filterChanged = function() { + $scope.filterChanged = function(isNew = true) { if (!$scope.filterItems) return; - let index = $scope.items.findIndex(item => { + let index = $scope.items.findIndex((item, i) => { + // Skip items that are before the previously selected index. + if (!isNew && i <= prevIndex) return false; + + // Find the next index such that all filters are satisfied. return $scope.filterItems.reduce((b, f) => { return b && f.filter(item, f.text); }, true); }); - if (index === -1) return; + if (index === -1) { + if (isNew) return; + index = firstFilteredIndex; + } $scope.selectItem({}, index); + if (isNew) firstFilteredIndex = index; }; $scope.$on('destroy', () => toggleEventListeners(false)); diff --git a/src/javascripts/Factories/hotkeyFactory.js b/src/javascripts/Factories/hotkeyFactory.js index 29a350a5..7a710662 100644 --- a/src/javascripts/Factories/hotkeyFactory.js +++ b/src/javascripts/Factories/hotkeyFactory.js @@ -282,14 +282,15 @@ ngapp.service('hotkeyFactory', function() { }] }; - let closeFilter = (scope, e) => { - e.stopPropagation(); - scope.toggleFilter(false); - }; - this.listViewFilterHotkeys = { - escape: closeFilter, - enter: closeFilter, + escape: (scope, e) => { + e.stopPropagation(); + scope.toggleFilter(false); + }, + enter: (scope, e) => { + e.stopPropagation(); + scope.filterChanged(false); + }, a: [{ modifiers: ['ctrlKey'], callback: (scope, e) => { From 263cb1d25873f22c984f8355d1692bb1588bd14f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 6 Nov 2021 14:37:24 -0700 Subject: [PATCH 2/9] Fix selecting 1st of prev filter when new filter has no matches --- src/javascripts/Directives/listView.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 308680f1..41d1c1ad 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -233,7 +233,11 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }, true); }); if (index === -1) { - if (isNew) return; + if (isNew || firstFilteredIndex === -1) { + firstFilteredIndex = -1; + return; + } + // The end has been reached; cycle back to the start. index = firstFilteredIndex; } $scope.selectItem({}, index); From eb6af3524fafde85503e1fd253985bb4bf35930a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 6 Nov 2021 14:55:27 -0700 Subject: [PATCH 3/9] Keep current selection if it matches the new filter --- src/javascripts/Directives/listView.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 41d1c1ad..5f1abe0d 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -51,6 +51,12 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk index === dragData.index; }; + let checkFilters = function(item) { + return $scope.filterItems.reduce((b, f) => { + return b && f.filter(item, f.text); + }, true); + } + // inherited variables and functions $scope.contextMenuItems = contextMenuFactory.checkboxListItems; hotkeyService.buildOnKeyDown($scope, 'onKeyDown', 'listView'); @@ -223,14 +229,19 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk $scope.filterChanged = function(isNew = true) { if (!$scope.filterItems) return; + + // If the current selection matches all filters, just keep it selected + // even if it's not necessarily the first possible match. + let prevMatches = isNew && prevIndex !== -1 && checkFilters($scope.items[prevIndex]); + + // Even if prevMatches is true, still search for the index of the first + // match so firstFilteredIndex can be set. let index = $scope.items.findIndex((item, i) => { // Skip items that are before the previously selected index. if (!isNew && i <= prevIndex) return false; // Find the next index such that all filters are satisfied. - return $scope.filterItems.reduce((b, f) => { - return b && f.filter(item, f.text); - }, true); + return checkFilters(item); }); if (index === -1) { if (isNew || firstFilteredIndex === -1) { @@ -240,7 +251,7 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk // The end has been reached; cycle back to the start. index = firstFilteredIndex; } - $scope.selectItem({}, index); + if (!prevMatches) $scope.selectItem({}, index); if (isNew) firstFilteredIndex = index; }; From a4f7c7c76b024117aeade99a1b16aef7acfec2d7 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 6 Nov 2021 18:48:54 -0700 Subject: [PATCH 4/9] Add checkbox to only show list view items matching the filters --- app/directives/listView.html | 6 +- src/javascripts/Directives/listView.js | 133 +++++++++++++++++++------ 2 files changed, 106 insertions(+), 33 deletions(-) diff --git a/app/directives/listView.html b/app/directives/listView.html index b6ee6f4a..765e2025 100644 --- a/app/directives/listView.html +++ b/app/directives/listView.html @@ -1,5 +1,5 @@
-
+
@@ -9,4 +9,8 @@
+ diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 5f1abe0d..ba91a1a9 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -14,6 +14,18 @@ ngapp.directive('listView', function() { } }); +ngapp.filter('listViewFilter', function() { + return function(items, filterItems, filterOptions) { + if (!filterOptions.onlyShowMatches) return items; + if (!filterItems) return items; + return items.filter(item => { + return filterItems.reduce((b, f) => { + return b && f.filter(item, f.text); + }, true); + }); + } +}); + ngapp.controller('listViewController', function($scope, $timeout, $element, hotkeyService, contextMenuService, contextMenuFactory, htmlHelpers) { // initialization $scope.parent = htmlHelpers.findParent($element[0], el => { @@ -21,10 +33,13 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }); $scope.listItems = $element[0].firstElementChild; + $scope.filteredItems = $scope.items; + $scope.filterOptions = { + onlyShowMatches: false + }; // helper variables - let prevIndex = -1, - firstFilteredIndex = -1, + let firstFilteredIndex = -1, eventListeners = { click: e => $scope.$apply(() => $scope.onParentClick(e)), keydown: e => $scope.$apply(() => { @@ -32,6 +47,30 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }) }; + const prevIndex = { + _value: -1, + _filteredValue: -1, + get value() { + return this._value; + }, + get filteredValue() { + return this._filteredValue; + }, + set filteredValue(i) { + this._filteredValue = i; + this._value = i > -1 ? $scope.filteredItems[i].index : -1; + } + } + + $scope.$watchCollection("filteredItems", function() { + if ($scope.filterOptions.onlyShowMatches) { + prevIndex.filteredValue = toFilteredIndex(prevIndex.value); + firstFilteredIndex = $scope.filteredItems.length > 0 ? 0 : -1; + } else { + prevIndex.filteredValue = prevIndex.value; + } + }); + // helper functions let removeClasses = function(element) { element.classList.remove('insert-after'); @@ -57,6 +96,12 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }, true); } + let toFilteredIndex = function(index) { + return $scope.filteredItems.findIndex(item => { + return item.index === index; + }); + } + // inherited variables and functions $scope.contextMenuItems = contextMenuFactory.checkboxListItems; hotkeyService.buildOnKeyDown($scope, 'onKeyDown', 'listView'); @@ -69,12 +114,13 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk $scope.clearSelection = function(resetPrevIndex) { $scope.items.forEach(item => item.selected = false); - if (resetPrevIndex) prevIndex = -1; + if (resetPrevIndex) prevIndex.filteredValue = -1; }; $scope.selectAll = function() { + if ($scope.filterOptions.onlyShowMatches) $scope.clearSelection(false); $scope.items.forEach(item => item.selected = true); - prevIndex = $scope.items.length - 1; + prevIndex.filteredValue = $scope.filteredItems.length - 1; }; $scope.toggleSelected = function(targetValue) { @@ -107,22 +153,22 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }; $scope.selectItem = function(e, index, scroll = true) { - let item = $scope.items[index]; + let item = $scope.filteredItems[index]; if (scroll) $scope.scrollTo(index); - if (e.shiftKey && prevIndex > -1) { - let start = Math.min(index, prevIndex), - end = Math.max(index, prevIndex); + if (e.shiftKey && prevIndex.filteredValue > -1) { + let start = Math.min(index, prevIndex.filteredValue), + end = Math.max(index, prevIndex.filteredValue); if (!e.ctrlKey) $scope.clearSelection(); for (let i = start; i <= end; i++) { - $scope.items[i].selected = true; + $scope.filteredItems[i].selected = true; } } else if (e.ctrlKey) { item.selected = !item.selected; - prevIndex = index; + prevIndex.filteredValue = index; } else { $scope.clearSelection(true); item.selected = true; - prevIndex = index; + prevIndex.filteredValue = index; } }; @@ -138,13 +184,13 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }; $scope.handleUpArrow = function() { - prevIndex = (prevIndex < 1 ? $scope.items.length : prevIndex) - 1; - $scope.selectItem({}, prevIndex); + prevIndex.filteredValue = (prevIndex.filteredValue < 1 ? $scope.filteredItems.length : prevIndex.filteredValue) - 1; + $scope.selectItem({}, prevIndex.filteredValue); }; $scope.handleDownArrow = function() { - prevIndex = (prevIndex >= $scope.items.length - 1 ? -1 : prevIndex) + 1; - $scope.selectItem({}, prevIndex); + prevIndex.filteredValue = (prevIndex.filteredValue >= $scope.filteredItems.length - 1 ? -1 : prevIndex.filteredValue) + 1; + $scope.selectItem({}, prevIndex.filteredValue); }; $scope.onParentClick = function(e) { @@ -156,7 +202,7 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }; $scope.onItemMouseDown = function(e, index) { - let item = $scope.items[index]; + let item = $scope.filteredItems[index]; if (e.button !== 2 || !item.selected) $scope.selectItem(e, index); if (e.button === 2) $scope.showContextMenu(e); }; @@ -167,7 +213,7 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk element: e.target, source: $scope.dragType, index: index, - getItem: () => $scope.items.splice(index, 1)[0] + getItem: () => $scope.items.splice($scope.filteredItems[index].index, 1)[0] }); return true; }; @@ -196,12 +242,12 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk if (!dragData || dragData.source !== $scope.dragType) return; if (onSameItem(dragData, e, index)) return; let after = e.offsetY > (e.target.offsetHeight / 2), - lengthBefore = $scope.items.length, + lengthBefore = $scope.filteredItems.length, movedItem = dragData.getItem(), - adjust = lengthBefore > $scope.items.length && index > dragData.index; + adjust = lengthBefore > $scope.filteredItems.length && index > dragData.index; removeClasses(e.target); - $scope.items.splice(index + after - adjust, 0, movedItem); - prevIndex = index + after - adjust; + $scope.items.splice($scope.filteredItems[index + after - adjust].index, 0, movedItem); + prevIndex.filteredValue = index + after - adjust; $scope.$emit('itemsReordered'); return true; }; @@ -230,19 +276,32 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk $scope.filterChanged = function(isNew = true) { if (!$scope.filterItems) return; + if ($scope.filterOptions.onlyShowMatches && !isNew) { + // It's not new, so firstFilteredIndex is already set. + // The next index doesn't need to be searched for; + // it's trivially +1 since only matching items are being shown. + if ($scope.filteredItems.length > 0) $scope.handleDownArrow(); + return; + } + // If the current selection matches all filters, just keep it selected // even if it's not necessarily the first possible match. - let prevMatches = isNew && prevIndex !== -1 && checkFilters($scope.items[prevIndex]); - - // Even if prevMatches is true, still search for the index of the first - // match so firstFilteredIndex can be set. - let index = $scope.items.findIndex((item, i) => { - // Skip items that are before the previously selected index. - if (!isNew && i <= prevIndex) return false; - - // Find the next index such that all filters are satisfied. - return checkFilters(item); - }); + let prevMatches = isNew && prevIndex.filteredValue !== -1 && checkFilters($scope.filteredItems[prevIndex.filteredValue]); + + // When only matches are shown, the first index is trivially 0. + // If all items are shown, then it does need to be searched for. + let index = $scope.filteredItems.length > 0 ? 0 : -1; + if (!$scope.filterOptions.onlyShowMatches) { + // Even if prevMatches is true, still search for the index of the first + // match so firstFilteredIndex can be set. + index = $scope.filteredItems.findIndex((item, i) => { + // Skip items that are before the previously selected index. + if (!isNew && i <= prevIndex.filteredValue) return false; + + // Find the next index such that all filters are satisfied. + return checkFilters(item); + }); + } if (index === -1) { if (isNew || firstFilteredIndex === -1) { firstFilteredIndex = -1; @@ -255,6 +314,16 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk if (isNew) firstFilteredIndex = index; }; + $scope.onOnlyShowMatchesChanged = function() { + // This executes before the filter is updated so that the + // firstFilteredIndex is still valid. + if (!$scope.filterOptions.onlyShowMatches) { + if (firstFilteredIndex > -1) { + firstFilteredIndex = $scope.filteredItems[firstFilteredIndex].index; + } + } + } + $scope.$on('destroy', () => toggleEventListeners(false)); $scope.$on('startDrag', function() { From 65b49de571d16d1bc0f73b78971637ccf3d1643d Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 11 Nov 2021 14:56:03 -0800 Subject: [PATCH 5/9] Move code for handling filter enter press into separate function --- src/javascripts/Directives/listView.js | 47 +++++++++++++--------- src/javascripts/Factories/hotkeyFactory.js | 2 +- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index ba91a1a9..8e78fa2c 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -273,20 +273,12 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk return true; }; - $scope.filterChanged = function(isNew = true) { + $scope.filterChanged = function() { if (!$scope.filterItems) return; - if ($scope.filterOptions.onlyShowMatches && !isNew) { - // It's not new, so firstFilteredIndex is already set. - // The next index doesn't need to be searched for; - // it's trivially +1 since only matching items are being shown. - if ($scope.filteredItems.length > 0) $scope.handleDownArrow(); - return; - } - // If the current selection matches all filters, just keep it selected // even if it's not necessarily the first possible match. - let prevMatches = isNew && prevIndex.filteredValue !== -1 && checkFilters($scope.filteredItems[prevIndex.filteredValue]); + let prevMatches = prevIndex.filteredValue !== -1 && checkFilters($scope.filteredItems[prevIndex.filteredValue]); // When only matches are shown, the first index is trivially 0. // If all items are shown, then it does need to be searched for. @@ -295,25 +287,40 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk // Even if prevMatches is true, still search for the index of the first // match so firstFilteredIndex can be set. index = $scope.filteredItems.findIndex((item, i) => { - // Skip items that are before the previously selected index. - if (!isNew && i <= prevIndex.filteredValue) return false; - // Find the next index such that all filters are satisfied. return checkFilters(item); }); } if (index === -1) { - if (isNew || firstFilteredIndex === -1) { - firstFilteredIndex = -1; - return; - } - // The end has been reached; cycle back to the start. - index = firstFilteredIndex; + firstFilteredIndex = -1; + return; } if (!prevMatches) $scope.selectItem({}, index); - if (isNew) firstFilteredIndex = index; + firstFilteredIndex = index; }; + $scope.selectNextFiltered = function() { + if (!$scope.filterItems || $scope.filteredItems.length === 0 || firstFilteredIndex === -1) return; + + if ($scope.filterOptions.onlyShowMatches) { + // The next index doesn't need to be searched for; + // it's trivially +1 since only matching items are being shown. + $scope.handleDownArrow(); + return; + } + + let index = $scope.filteredItems.findIndex((item, i) => { + // Find the next index such that all filters are satisfied. + // Skip items that are at/before the previously selected index. + return i > prevIndex.filteredValue && checkFilters(item); + }); + + // The end has been reached; cycle back to the start. + if (index === -1) index = firstFilteredIndex; + + $scope.selectItem({}, index); + } + $scope.onOnlyShowMatchesChanged = function() { // This executes before the filter is updated so that the // firstFilteredIndex is still valid. diff --git a/src/javascripts/Factories/hotkeyFactory.js b/src/javascripts/Factories/hotkeyFactory.js index 7a710662..d10d9092 100644 --- a/src/javascripts/Factories/hotkeyFactory.js +++ b/src/javascripts/Factories/hotkeyFactory.js @@ -289,7 +289,7 @@ ngapp.service('hotkeyFactory', function() { }, enter: (scope, e) => { e.stopPropagation(); - scope.filterChanged(false); + scope.selectNextFiltered(); }, a: [{ modifiers: ['ctrlKey'], From 5b8f81971155e8e879abb87d435c93b0ad63e4e0 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 11 Nov 2021 17:09:38 -0800 Subject: [PATCH 6/9] Fix incorrect item selected when filter changes filterChanged was only executing before the filteredItems array got updated. Thus, the index it was finding was not necessarily going to be accurate once filteredItems was updated. --- src/javascripts/Directives/listView.js | 50 ++++++++++++++++---------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 8e78fa2c..2f96f1fc 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -65,10 +65,16 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk $scope.$watchCollection("filteredItems", function() { if ($scope.filterOptions.onlyShowMatches) { prevIndex.filteredValue = toFilteredIndex(prevIndex.value); + + // When only matches are shown, the first index is trivially 0. firstFilteredIndex = $scope.filteredItems.length > 0 ? 0 : -1; } else { prevIndex.filteredValue = prevIndex.value; } + + // Can't know if the change happened cause of onlyShowMatches or + // filteredItems, so call it always. + $scope.filterChanged(); }); // helper functions @@ -274,29 +280,35 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk }; $scope.filterChanged = function() { - if (!$scope.filterItems) return; + let prevMatches = false; - // If the current selection matches all filters, just keep it selected - // even if it's not necessarily the first possible match. - let prevMatches = prevIndex.filteredValue !== -1 && checkFilters($scope.filteredItems[prevIndex.filteredValue]); + if ($scope.filterOptions.onlyShowMatches) { + // When only matches are shown, the first index is trivially 0. + firstFilteredIndex = $scope.filteredItems.length > 0 ? 0 : -1; - // When only matches are shown, the first index is trivially 0. - // If all items are shown, then it does need to be searched for. - let index = $scope.filteredItems.length > 0 ? 0 : -1; - if (!$scope.filterOptions.onlyShowMatches) { - // Even if prevMatches is true, still search for the index of the first - // match so firstFilteredIndex can be set. - index = $scope.filteredItems.findIndex((item, i) => { - // Find the next index such that all filters are satisfied. - return checkFilters(item); - }); + if (!$scope.filterItems || firstFilteredIndex === -1) return; + + prevMatches = prevIndex.filteredValue !== -1; + } else { + if (!$scope.filterItems) return; + + firstFilteredIndex = $scope.filteredItems.findIndex(checkFilters); + if (firstFilteredIndex === -1) return; + + // If the first index is the previous index, then it's already known + // that the previous index is a match. + prevMatches = firstFilteredIndex === prevIndex.value || (prevIndex.value !== -1 && checkFilters($scope.items[prevIndex.value])); } - if (index === -1) { - firstFilteredIndex = -1; - return; + + // Don't select the item if the previous selection already matches, + // or if the found index is already selected. + if (!prevMatches && !$scope.filteredItems[firstFilteredIndex].selected) { + $scope.selectItem({}, firstFilteredIndex); // This also calls scrollTo. + } else { + // Even if the selection remains, it may have gone out of view if + // onlyShowMatches was toggled. + $scope.scrollTo(firstFilteredIndex); } - if (!prevMatches) $scope.selectItem({}, index); - firstFilteredIndex = index; }; $scope.selectNextFiltered = function() { From a7d782298c688a08d82e3d00d2a655f090760ed1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 21 Nov 2021 20:01:29 -0800 Subject: [PATCH 7/9] Fix TypeError when dragging an item to the end of the list --- src/javascripts/Directives/listView.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 2f96f1fc..5145607b 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -249,10 +249,16 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk if (onSameItem(dragData, e, index)) return; let after = e.offsetY > (e.target.offsetHeight / 2), lengthBefore = $scope.filteredItems.length, - movedItem = dragData.getItem(), + movedItem = dragData.getItem(), // The item is removed in-place. adjust = lengthBefore > $scope.filteredItems.length && index > dragData.index; removeClasses(e.target); - $scope.items.splice($scope.filteredItems[index + after - adjust].index, 0, movedItem); + // Translate the index to the one in the unfiltered items array. If the + // destination index is the end of the array, then there won't be an + // item at that index within filteredItems. In such case, the translated + // index should just be the last index in the items array + 1 + // i.e. the length of the items array. + const spliceStart = index < $scope.filteredItems.length ? $scope.filteredItems[index].index : $scope.items.length; + $scope.items.splice(spliceStart + after - adjust, 0, movedItem); prevIndex.filteredValue = index + after - adjust; $scope.$emit('itemsReordered'); return true; From cfa1cb9d3f05cee712a117e45f9524ead9f7d1b9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 21 Nov 2021 20:31:32 -0800 Subject: [PATCH 8/9] Fix incorrect next selection after selected item is dragged For example, take 3 items named 1, 2, and 3. If 2 is dragged so that the order becomes 2, 1, 3, then the next item being selected was 3 rather than 1. This was because the `filteredValue` setter was setting the unfiltered `value` using the `index` property of the item in the array, but `itemsReordered` had not been handled yet. This meant that the `index` was still the item's previous position. When the watchCollection for `filteredItems` was triggered due to the drag causing a re-ordering, it was setting `filteredValue` to `value`, which had the wrong index. Fix this by setting `filteredValue` after `itemsReordered` is handled. --- src/javascripts/Directives/listView.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 5145607b..15fb50ca 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -259,8 +259,11 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk // i.e. the length of the items array. const spliceStart = index < $scope.filteredItems.length ? $scope.filteredItems[index].index : $scope.items.length; $scope.items.splice(spliceStart + after - adjust, 0, movedItem); - prevIndex.filteredValue = index + after - adjust; $scope.$emit('itemsReordered'); + // Unfortunately, execution order is important here. + // This must happen after itemsReordered is handled. + // The setter relies on the `index` property of the item being updated. + prevIndex.filteredValue = index + after - adjust; return true; }; From 490451b8f394ede0dff215370852202b1495e0fb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 21 Nov 2021 20:47:30 -0800 Subject: [PATCH 9/9] Fix TypeError when dragging item to bottom when only matches shown The `adjust` variable in `onItemDrop` was incorrectly evaluating to false because the length comparison used `filteredItems`'s lengths. At the time of comparison, that array is not updated yet to reflect the change caused by `getItem`. --- src/javascripts/Directives/listView.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/javascripts/Directives/listView.js b/src/javascripts/Directives/listView.js index 15fb50ca..d10e922a 100644 --- a/src/javascripts/Directives/listView.js +++ b/src/javascripts/Directives/listView.js @@ -248,15 +248,24 @@ ngapp.controller('listViewController', function($scope, $timeout, $element, hotk if (!dragData || dragData.source !== $scope.dragType) return; if (onSameItem(dragData, e, index)) return; let after = e.offsetY > (e.target.offsetHeight / 2), - lengthBefore = $scope.filteredItems.length, + // This and adjust cannot use filteredItem's length because that + // array will still not be updated yet after getItem is called. + lengthBefore = $scope.items.length, movedItem = dragData.getItem(), // The item is removed in-place. - adjust = lengthBefore > $scope.filteredItems.length && index > dragData.index; + adjust = lengthBefore > $scope.items.length && index > dragData.index; removeClasses(e.target); // Translate the index to the one in the unfiltered items array. If the // destination index is the end of the array, then there won't be an // item at that index within filteredItems. In such case, the translated // index should just be the last index in the items array + 1 // i.e. the length of the items array. + // + // It may seem like items.length is wrong to use when onlyShowMatches + // is true. However, when it's true, the item at that index will still + // exist in filteredItems because the array has not been updated yet to + // reflect the deletion caused by getItem(). It does reflect the change + // when onlyShowMatches is false because filteredItems then refers to + // the same array instance as `items`. const spliceStart = index < $scope.filteredItems.length ? $scope.filteredItems[index].index : $scope.items.length; $scope.items.splice(spliceStart + after - adjust, 0, movedItem); $scope.$emit('itemsReordered');