From 1096c53f49625731763ff4f5e11bf5b9e8febb64 Mon Sep 17 00:00:00 2001
From: Stefan Zermatten <stefanzermatten@gmail.com>
Date: Mon, 5 Jun 2023 15:44:53 +0200
Subject: [PATCH] Improved slot filling UI usability

---
 .../methods/getSlotFillFilter.js              |   6 +-
 .../methods/getSlotFillFilter.test.js         |   1 -
 .../computeSlotQuantityFilled.js              |   5 +-
 .../computeByType/computeSlot.js              |   4 +-
 .../api/library/getCreatureLibraryIds.js      |   5 +-
 app/imports/api/utility/escapeRegex.js        |   3 +
 .../ui/creature/slots/SlotFillDialog.vue      | 198 +++++++++++-------
 .../library/LibraryNodeExpansionContent.vue   |  38 +++-
 app/imports/server/publications/library.js    |   7 +-
 .../server/publications/searchLibraryNodes.js |   5 +-
 .../server/publications/slotFillers.js        |  35 ++--
 11 files changed, 182 insertions(+), 125 deletions(-)
 create mode 100644 app/imports/api/utility/escapeRegex.js

diff --git a/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.js b/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.js
index a25e7bf4..f6a5e7f9 100644
--- a/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.js
+++ b/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.js
@@ -1,7 +1,7 @@
 export default function getSlotFillFilter({ slot, libraryIds }) {
 
-  if (!slot) throw 'Slot is required';
-  if (!libraryIds) throw 'LibraryIds is required';
+  if (!slot) throw 'Slot is required for getSlotFillFilter';
+  if (!libraryIds) throw 'LibraryIds is required for getSlotFillFilter';
 
   let filter = {
     removed: { $ne: true },
@@ -13,7 +13,6 @@ export default function getSlotFillFilter({ slot, libraryIds }) {
       $or: [{
         type: slot.slotType
       }, {
-        type: 'slotFiller',
         slotFillerType: slot.slotType,
       }]
     });
@@ -22,7 +21,6 @@ export default function getSlotFillFilter({ slot, libraryIds }) {
       $or: [{
         type: 'classLevel',
       }, {
-        type: 'slotFiller',
         slotFillerType: 'classLevel',
       }]
     });
diff --git a/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.test.js b/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.test.js
index 2e4048d2..5ca0f4ed 100644
--- a/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.test.js
+++ b/app/imports/api/creature/creatureProperties/methods/getSlotFillFilter.test.js
@@ -50,7 +50,6 @@ describe('Slot fill filter', function () {
       $or: [{
         type: 'feature'
       }, {
-        type: 'slotFiller',
         slotFillerType: 'feature',
       }],
     }]);
diff --git a/app/imports/api/engine/computation/buildComputation/computeSlotQuantityFilled.js b/app/imports/api/engine/computation/buildComputation/computeSlotQuantityFilled.js
index e3adabc8..e9e4a287 100644
--- a/app/imports/api/engine/computation/buildComputation/computeSlotQuantityFilled.js
+++ b/app/imports/api/engine/computation/buildComputation/computeSlotQuantityFilled.js
@@ -2,7 +2,7 @@
  * Only computes `totalFilled`, need to compute `quantityExpected.value`
  * before `spacesLeft` can be computed
  */
-export default function computeSlotQuantityFilled(node, dependencyGraph){
+export default function computeSlotQuantityFilled(node, dependencyGraph) {
   let slot = node.node;
   if (slot.type !== 'propertySlot') return;
   slot.totalFilled = 0;
@@ -10,9 +10,8 @@ export default function computeSlotQuantityFilled(node, dependencyGraph){
     let childProp = child.node;
     dependencyGraph.addLink(slot._id, childProp._id, 'slotFill');
     if (
-      childProp.type === 'slotFiller' &&
       Number.isFinite(childProp.slotQuantityFilled)
-    ){
+    ) {
       slot.totalFilled += childProp.slotQuantityFilled;
     } else {
       slot.totalFilled++;
diff --git a/app/imports/api/engine/computation/computeComputation/computeByType/computeSlot.js b/app/imports/api/engine/computation/computeComputation/computeByType/computeSlot.js
index 9e9dc07b..8128719a 100644
--- a/app/imports/api/engine/computation/computeComputation/computeByType/computeSlot.js
+++ b/app/imports/api/engine/computation/computeComputation/computeByType/computeSlot.js
@@ -1,6 +1,6 @@
-export default function computSlot(computation, node){
+export default function computeSlot(computation, node) {
   const prop = node.data;
-  if (prop.quantityExpected && prop.quantityExpected.value){
+  if (prop.quantityExpected && prop.quantityExpected.value) {
     prop.spaceLeft = prop.quantityExpected.value - prop.totalFilled;
   }
 }
diff --git a/app/imports/api/library/getCreatureLibraryIds.js b/app/imports/api/library/getCreatureLibraryIds.js
index beb9c7d3..a951a042 100644
--- a/app/imports/api/library/getCreatureLibraryIds.js
+++ b/app/imports/api/library/getCreatureLibraryIds.js
@@ -4,6 +4,7 @@ import getUserLibraryIds from './getUserLibraryIds';
 import { intersection, union } from 'lodash';
 
 export default function getCreatureLibraryIds(creature, userId) {
+  if (!userId) console.log('no userId, returning empty array');
   if (!userId) return [];
 
   // Get the ids of libraries the user is permitted to view
@@ -17,14 +18,14 @@ export default function getCreatureLibraryIds(creature, userId) {
         allowedLibraryCollections: 1,
       }
     });
-    if (!creature) return [];
+    if (!creature) return userLibIds;
   }
 
   // If the creature does not restrict the libraries, let it use them all
   if (!creature.allowedLibraryCollections && !creature.allowedLibraries) {
     return userLibIds;
   }
-  
+
   // Get the ids of the libraries that the creature allows
   const allowedCollections = creature.allowedLibraryCollections || [];
   let creatureLibIds = creature.allowedLibraries || [];
diff --git a/app/imports/api/utility/escapeRegex.js b/app/imports/api/utility/escapeRegex.js
new file mode 100644
index 00000000..8ec4073c
--- /dev/null
+++ b/app/imports/api/utility/escapeRegex.js
@@ -0,0 +1,3 @@
+export default function escapeRegex(string) {
+  return string.replace(/[/\-\\^$*+?.()|[\]{}]/g, '');
+}
\ No newline at end of file
diff --git a/app/imports/client/ui/creature/slots/SlotFillDialog.vue b/app/imports/client/ui/creature/slots/SlotFillDialog.vue
index 8b2d24f0..15fc925b 100644
--- a/app/imports/client/ui/creature/slots/SlotFillDialog.vue
+++ b/app/imports/client/ui/creature/slots/SlotFillDialog.vue
@@ -26,7 +26,7 @@
       :string="model.description"
     />
     <p>
-      {{ slotPropertyTypeName }} with tags:
+      {{ slotPropertyTypeName }} with library tags:
       <property-tags
         v-for="(tags, index) in tagsSearched.or"
         :key="index + 'tags'"
@@ -40,82 +40,97 @@
         prefix="NOT"
       />
     </p>
-    <v-expansion-panels
-      multiple
-      inset
-    >
-      <template v-for="libraryNode in libraryNodes">
-        <v-expansion-panel
-          v-if="showDisabled || !libraryNode._disabledBySlotFillerCondition"
-          :key="libraryNode._id"
-          :model="libraryNode"
-          :data-id="libraryNode._id"
-          :class="{disabled: isDisabled(libraryNode)}"
-        >
-          <v-expansion-panel-header>
-            <template #default="{ open }">
-              <v-layout
-                align-center
-                class="flex-grow-0 mr-2"
-              >
-                <v-checkbox
-                  v-if="libraryNode._disabledByAlreadyAdded"
-                  class="my-0 py-0"
-                  hide-details
-                  :input-value="true"
-                  disabled
-                />
-                <v-checkbox
-                  v-else
-                  v-model="selectedNodeIds"
-                  class="my-0 py-0"
-                  hide-details
-                  :disabled="isDisabled(libraryNode)"
-                  :value="libraryNode._id"
-                  @click.stop
-                />
-              </v-layout>
-              <v-layout column>
-                <v-layout align-center>
-                  <tree-node-view :model="libraryNode" />
-                  <div
-                    v-if="libraryNode._disabledBySlotFillerCondition"
-                    class="error--text text-no-wrap text-truncate"
-                  >
-                    {{ libraryNode.slotFillerCondition }}
+    <v-fade-transition>
+      <div
+        v-if="!$subReady.slotFillers"
+        class="fill-height layout justify-center align-center"
+      >
+        <v-progress-circular
+          indeterminate
+          color="primary"
+          size="64"
+        />
+      </div>
+      <v-expansion-panels
+        v-else
+        accordion
+        tile
+        multiple
+        hover
+      >
+        <template v-for="libraryNode in libraryNodes">
+          <v-expansion-panel
+            v-if="showDisabled || !libraryNode._disabledBySlotFillerCondition"
+            :key="libraryNode._id"
+            :model="libraryNode"
+            :data-id="libraryNode._id"
+            :class="{disabled: isDisabled(libraryNode)}"
+          >
+            <v-expansion-panel-header>
+              <template #default="{ open }">
+                <v-layout
+                  align-center
+                  class="flex-grow-0 mr-2"
+                >
+                  <v-checkbox
+                    v-if="libraryNode._disabledByAlreadyAdded"
+                    class="my-0 py-0"
+                    hide-details
+                    :input-value="true"
+                    disabled
+                  />
+                  <v-checkbox
+                    v-else
+                    v-model="selectedNodeIds"
+                    class="my-0 py-0"
+                    hide-details
+                    :disabled="isDisabled(libraryNode)"
+                    :value="libraryNode._id"
+                    @click.stop
+                  />
+                </v-layout>
+                <v-layout column>
+                  <v-layout align-center>
+                    <tree-node-view :model="libraryNode" />
+                    <div
+                      v-if="libraryNode._disabledBySlotFillerCondition"
+                      class="error--text text-no-wrap text-truncate"
+                    >
+                      {{ libraryNode._conditionError }}
+                    </div>
+                  </v-layout>
+                  <div class="text-caption text-no-wrap text-truncate">
+                    {{ libraryNames[libraryNode.ancestors[0].id ] }}
                   </div>
                 </v-layout>
-                <div class="text-caption text-no-wrap text-truncate">
-                  {{ libraryNames[libraryNode.ancestors[0].id ] }}
-                </div>
-              </v-layout>
-              <div
-                v-if="libraryNode.slotQuantityFilled !== undefined && libraryNode.slotQuantityFilled !== 1"
-                class="text-overline flex-grow-0 text-no-wrap"
-                :class="{
-                  'error--text': isDisabled(libraryNode) &&
-                    libraryNode._disabledByQuantityFilled
-                }"
-              >
-                {{ libraryNode.slotQuantityFilled }} slots
-              </div>
-              <template v-if="open">
-                <v-btn
-                  icon
-                  class="flex-grow-0"
-                  @click.stop="openPropertyDetails(libraryNode._id)"
+                <div
+                  v-if="libraryNode.slotQuantityFilled !== undefined && libraryNode.slotQuantityFilled !== 1"
+                  class="text-overline flex-grow-0 text-no-wrap"
+                  :class="{
+                    'error--text': isDisabled(libraryNode) &&
+                      libraryNode._disabledByQuantityFilled
+                  }"
                 >
-                  <v-icon>mdi-window-restore</v-icon>
-                </v-btn>
+                  {{ libraryNode.slotQuantityFilled }} slots
+                </div>
+                <template v-if="open">
+                  <v-btn
+                    icon
+                    class="flex-grow-0"
+                    @click.stop="openPropertyDetails(libraryNode._id)"
+                  >
+                    <v-icon>mdi-window-restore</v-icon>
+                  </v-btn>
+                </template>
               </template>
-            </template>
-          </v-expansion-panel-header>
-          <v-expansion-panel-content>
-            <library-node-expansion-content :model="libraryNode" />
-          </v-expansion-panel-content>
-        </v-expansion-panel>
-      </template>
-    </v-expansion-panels>
+            </v-expansion-panel-header>
+            <v-expansion-panel-content>
+              <library-node-expansion-content :model="libraryNode" />
+            </v-expansion-panel-content>
+          </v-expansion-panel>
+        </template>
+      </v-expansion-panels>
+    </v-fade-transition>
     <v-layout
       v-if="(!$subReady.slotFillers && !searchValue) || currentLimit < countAll"
       column
@@ -188,8 +203,6 @@ import TreeNodeView from '/imports/client/ui/properties/treeNodeViews/TreeNodeVi
 import PropertyDescription from '/imports/client/ui/properties/viewers/shared/PropertyDescription.vue'
 import resolve, { toString } from '/imports/parser/resolve.js';
 import { prettifyParseError, parse } from '/imports/parser/parser.js';
-// import evaluateString from '/imports/api/creature/computation/afterComputation/evaluateString.js';
-import getSlotFillFilter from '/imports/api/creature/creatureProperties/methods/getSlotFillFilter.js'
 import Libraries from '/imports/api/library/Libraries.js';
 import LibraryNodeExpansionContent from '/imports/client/ui/library/LibraryNodeExpansionContent.vue';
 import PropertyTags from '/imports/client/ui/properties/viewers/shared/PropertyTags.vue';
@@ -225,6 +238,7 @@ export default {
       searchValue: undefined,
       showDisabled: false,
       disabledNodeCount: undefined,
+      autoSelectRan: false,
     }
   },
   reactiveProvide: {
@@ -309,6 +323,11 @@ export default {
     countAll() {
       return this._subs['slotFillers'].data('countAll');
     },
+    libraryNodeFilter() {
+      const filterString = this._subs['slotFillers'].data('libraryNodeFilter');
+      if (!filterString) return;
+      return EJSON.parse(filterString);
+    },
     alreadyAdded() {
       let added = new Set();
       if (!this.model.unique) return added;
@@ -354,8 +373,9 @@ export default {
       return names;
     },
     libraryNodes() {
-      let filter = getSlotFillFilter({ slot: this.model });
-      let nodes = LibraryNodes.find(filter, {
+      if (!this.libraryNodeFilter) return [];
+      if (!this.$subReady.slotFillers) return [];
+      let nodes = LibraryNodes.find(this.libraryNodeFilter, {
         sort: { name: 1, order: 1 }
       }).fetch();
       let disabledNodeCount = 0;
@@ -368,32 +388,50 @@ export default {
             const { result: resultNode } = resolve('reduce', parseNode, this.variables);
             if (resultNode?.parseType === 'constant') {
               if (!resultNode.value) {
+                node._disabled = true;
                 node._disabledBySlotFillerCondition = true;
+                node._conditionError = node.slotFillerConditionNote || node.slotFillerCondition;
                 disabledNodeCount += 1;
               }
             } else {
+              node._disabled = true;
               node._disabledBySlotFillerCondition = true;
-              node._conditionError = toString(resultNode);
+              node._conditionError = node.slotFillerConditionNote || toString(resultNode);
               disabledNodeCount += 1;
             }
           } catch (e) {
             console.warn(e);
             let error = prettifyParseError(e);
+            node._disabled = true;
             node._disabledBySlotFillerCondition = true;
-            node._conditionError = error;
+            node._conditionError = 'Condition error: '+ error;
             disabledNodeCount += 1;
           }
         }
-        let quantityToFill = node.type === 'slotFiller' ? node.slotQuantityFilled : 1;
+        let quantityToFill = typeof node.slotQuantityFilled == 'number' ? node.slotQuantityFilled : 1;
         if (
           quantityToFill > this.spaceLeft
         ) {
+          node._disabled = true;
           node._disabledByQuantityFilled = true;
         }
         if (this.alreadyAdded.has(node._id)) {
+          node._disabled = true;
           node._disabledByAlreadyAdded = true;
         }
       });
+      // Only run the auto-select once
+      if (!this.autoSelectRan) {
+        this.autoSelectRan = true;
+        // If we have exactly one active node and no selected nodes, pre-select it
+        if (
+          nodes.length === 1
+          && !nodes[0]._disabled
+          && !this.selectedNodeIds?.length
+        ) {
+          this.selectedNodeIds = [nodes[0]._id];
+        }
+      }
       this.disabledNodeCount = disabledNodeCount;
       return nodes;
     },
diff --git a/app/imports/client/ui/library/LibraryNodeExpansionContent.vue b/app/imports/client/ui/library/LibraryNodeExpansionContent.vue
index 1067d4a5..f6168488 100644
--- a/app/imports/client/ui/library/LibraryNodeExpansionContent.vue
+++ b/app/imports/client/ui/library/LibraryNodeExpansionContent.vue
@@ -1,16 +1,27 @@
 <template lang="html">
   <div>
-    <component
-      :is="model.type"
-      :model="model"
-      class="property-viewer"
-    />
-    <tree-node-list
-      v-if="$subReady.descendantLibraryNodes"
-      group="library-node-expansion"
-      :children="propertyChildren"
-      @selected="clickChild"
+    <v-progress-linear
+      v-if="!subsReady"
+      indeterminate
+      color="accent"
     />
+    <v-expand-transition>
+      <div
+        v-if="subsReady"
+        class="pt-4"
+      >
+        <component
+          :is="model.type"
+          :model="model"
+          class="property-viewer"
+        />
+        <tree-node-list
+          group="library-node-expansion"
+          :children="propertyChildren"
+          @selected="clickChild"
+        />
+      </div>
+    </v-expand-transition>
   </div>
 </template>
 
@@ -31,6 +42,11 @@ export default {
       required: true,
     },
   },
+  computed: {
+    subsReady() {
+      return this.$subReady.descendantLibraryNodes && this.$subReady.libraryNode;
+    }
+  },
   methods: {
     clickChild(id){
       this.$store.commit('pushDialogStack', {
@@ -57,7 +73,7 @@ export default {
         ancestorId: this.model._id
       });
     },
-  },
+  }
 }
 </script>
 
diff --git a/app/imports/server/publications/library.js b/app/imports/server/publications/library.js
index 68c378b5..5fe0b019 100644
--- a/app/imports/server/publications/library.js
+++ b/app/imports/server/publications/library.js
@@ -14,15 +14,18 @@ const LIBRARY_NODE_TREE_FIELDS = {
   order: 1,
   parent: 1,
   ancestors: 1,
-  tags: 1,
-  slotFillerCondition: 1,
   removed: 1,
   removedAt: 1,
   // Actions
   actionType: 1,
   // SlotFillers
+  libraryTags: 1,
   slotQuantityFilled: 1,
   slotFillerType: 1,
+  slotFillerConditionNote: 1,
+  slotFillerCondition: 1,
+  searchable: 1,
+  slotFillImage: 1,
   // Effect
   operation: 1,
   targetTags: 1,
diff --git a/app/imports/server/publications/searchLibraryNodes.js b/app/imports/server/publications/searchLibraryNodes.js
index bbfcea84..75750d33 100644
--- a/app/imports/server/publications/searchLibraryNodes.js
+++ b/app/imports/server/publications/searchLibraryNodes.js
@@ -4,10 +4,7 @@ import LibraryNodes from '/imports/api/library/LibraryNodes.js';
 import getCreatureLibraryIds from '/imports/api/library/getCreatureLibraryIds.js';
 import getUserLibraryIds from '/imports/api/library/getUserLibraryIds.js';
 import { assertViewPermission } from '/imports/api/sharing/sharingPermissions.js';
-
-function escapeRegex(string) {
-  return string.replace(/[/\-\\^$*+?.()|[\]{}]/g, '');
-}
+import escapeRegex from '/imports/api/utility/escapeRegex.js';
 
 Meteor.publish('selectedLibraryNodes', function (selectedNodeIds) {
   check(selectedNodeIds, Array);
diff --git a/app/imports/server/publications/slotFillers.js b/app/imports/server/publications/slotFillers.js
index 6427af79..d3cf32c0 100644
--- a/app/imports/server/publications/slotFillers.js
+++ b/app/imports/server/publications/slotFillers.js
@@ -5,19 +5,20 @@ import CreatureProperties from '/imports/api/creature/creatureProperties/Creatur
 import getSlotFillFilter from '/imports/api/creature/creatureProperties/methods/getSlotFillFilter.js'
 import getCreatureLibraryIds from '/imports/api/library/getCreatureLibraryIds.js';
 import { LIBRARY_NODE_TREE_FIELDS } from '/imports/server/publications/library.js';
+import escapeRegex from '/imports/api/utility/escapeRegex.js';
 
-Meteor.publish('slotFillers', function(slotId, searchTerm){
+Meteor.publish('slotFillers', function (slotId, searchTerm) {
   if (searchTerm) check(searchTerm, String);
 
   let self = this;
-  this.autorun(function (){
+  this.autorun(function () {
     let userId = this.userId;
     if (!userId) {
       return [];
     }
     // Get the slot
     let slot = CreatureProperties.findOne(slotId);
-    if (!slot){
+    if (!slot) {
       return [];
     }
 
@@ -36,31 +37,32 @@ Meteor.publish('slotFillers', function(slotId, searchTerm){
     });
 
     // Build a filter for nodes in those libraries that match the slot
-    let filter = getSlotFillFilter({slot, libraryIds});
-
-    this.autorun(function(){
+    let filter = getSlotFillFilter({ slot, libraryIds });
+    this.autorun(function () {
       // Get the limit of the documents the user can fetch
       var limit = self.data('limit') || 50;
       check(limit, Number);
 
       let options = undefined;
-      if (searchTerm){
-        filter.$text = {$search: searchTerm};
+      if (searchTerm) {
+        filter.name = { $regex: escapeRegex(searchTerm), '$options': 'i' };
+        //filter.$text = { $search: searchTerm };
         options = {
           // relevant documents have a higher score.
           fields: {
-            _score: { $meta: 'textScore' },
+            //_score: { $meta: 'textScore' },
             ...LIBRARY_NODE_TREE_FIELDS,
           },
           sort: {
             // `score` property specified in the projection fields above.
-            _score: { $meta: 'textScore' },
+            //_score: { $meta: 'textScore' },
             name: 1,
             order: 1,
           }
         }
       } else {
-        delete filter.$text
+        //delete filter.$text
+        delete filter.name
         options = {
           sort: {
             name: 1,
@@ -73,6 +75,7 @@ Meteor.publish('slotFillers', function(slotId, searchTerm){
 
       self.autorun(function () {
         self.setData('countAll', LibraryNodes.find(filter).count());
+        self.setData('libraryNodeFilter', EJSON.stringify(filter));
       });
       self.autorun(function () {
         return [
@@ -84,18 +87,18 @@ Meteor.publish('slotFillers', function(slotId, searchTerm){
   });
 });
 
-Meteor.publish('classFillers', function(classId){
+Meteor.publish('classFillers', function (classId) {
   let self = this;
   if (!classId) return [];
 
-  this.autorun(function (){
+  this.autorun(function () {
     let userId = this.userId;
     if (!userId) {
       return [];
     }
     // Get the class
     let classProp = CreatureProperties.findOne(classId);
-    if (!classProp){
+    if (!classProp) {
       return [];
     }
 
@@ -114,9 +117,9 @@ Meteor.publish('classFillers', function(classId){
     });
 
     // Build a filter for nodes in those libraries that match the slot
-    let filter = getSlotFillFilter({slot: classProp, libraryIds});
+    let filter = getSlotFillFilter({ slot: classProp, libraryIds });
 
-    this.autorun(function(){
+    this.autorun(function () {
       // Get the limit of the documents the user can fetch
       var limit = self.data('limit') || 50;
       check(limit, Number);