Fix reprint loop on tag filters

- Fix endless loop if a tag was selected
- Tag selection is now saved, both 'selected' and 'excluded' (old state is lost though)
- Streamlined reprinting even more by refactoring bogus drilldown

public/scripts/filters.js

@@ -24,6 +24,7 @@ export const FILTER_STATES = {
     EXCLUDED: { key: 'EXCLUDED', class: 'excluded' },
     UNDEFINED: { key: 'UNDEFINED', class: 'undefined' },
 };
+export const DEFAULT_FILTER_STATE = FILTER_STATES.UNDEFINED.key;
 
 /**
  * Robust check if one state equals the other. It does not care whether it's the state key or the state value object.
@@ -203,7 +204,7 @@ export class FilterHelper {
         return this.filterDataByState(data, state, isFolder);
     }
 
-    filterDataByState(data, state, filterFunc, { includeFolders } = {}) {
+    filterDataByState(data, state, filterFunc, { includeFolders = false } = {}) {
         if (isFilterState(state, FILTER_STATES.SELECTED)) {
             return data.filter(entity => filterFunc(entity) || (includeFolders && entity.type == 'tag'));
         }

public/scripts/tags.js

@@ -13,7 +13,7 @@ import {
     event_types,
 } from '../script.js';
 // eslint-disable-next-line no-unused-vars
-import { FILTER_TYPES, FILTER_STATES, isFilterState, FilterHelper } from './filters.js';
+import { FILTER_TYPES, FILTER_STATES, DEFAULT_FILTER_STATE, isFilterState, FilterHelper } from './filters.js';
 
 import { groupCandidatesFilter, groups, selected_group } from './group-chats.js';
 import { download, onlyUnique, parseJsonFile, uuidv4, getSortableDelay, debounce } from './utils.js';
@@ -180,6 +180,7 @@ function isBogusFolderOpen() {
 
 /**
  * Function to be called when a specific tag/folder is chosen to "drill down".
+ *
  * @param {*} source The jQuery element clicked when choosing the folder
  * @param {string} tagId The tag id that is behind the chosen folder
  * @param {boolean} remove Whether the given tag should be removed (otherwise it is added/chosen)
@@ -197,12 +198,9 @@ function chooseBogusFolder(source, tagId, remove = false) {
     // Instead of manually updating the filter conditions, we just "click" on the filter tag
     // We search inside which filter block we are located in and use that one
     const FILTER_SELECTOR = ($(source).closest('#rm_characters_block') ?? $(source).closest('#rm_group_chats_block')).find('.rm_tag_filter');
-    if (remove) {
+    const tagElement = $(FILTER_SELECTOR).find(`.tag[id=${tagId}]`);
-        // Click twice to skip over the 'excluded' state
+
-        $(FILTER_SELECTOR).find(`.tag[id=${tagId}]`).trigger('click').trigger('click');
+    toggleTagThreeState(tagElement, { stateOverride: DEFAULT_FILTER_STATE, simulateClick: true });
-    } else {
-        $(FILTER_SELECTOR).find(`.tag[id=${tagId}]`).trigger('click');
-    }
 }
 
 /**
@@ -603,8 +601,8 @@ function appendTagToList(listElement, tag, { removable = false, selectable = fal
         tagElement.find('.tag_name').text('').attr('title', tag.name).addClass(tag.icon);
     }
 
-    if (tag.excluded && isGeneralList) {
+    if (selectable && isGeneralList) {
-        toggleTagThreeState(tagElement, { stateOverride: FILTER_STATES.EXCLUDED });
+        toggleTagThreeState(tagElement, { stateOverride: tag.filterState ?? DEFAULT_FILTER_STATE });
     }
 
     if (selectable) {
@@ -629,34 +627,28 @@ function onTagFilterClick(listElement) {
 
     let state = toggleTagThreeState($(this));
 
-    // Manual undefined check required for three-state boolean
     if (existingTag) {
-        existingTag.excluded = isFilterState(state, FILTER_STATES.EXCLUDED);
+        existingTag.filterState = state;
-
         saveSettingsDebounced();
     }
 
-    // Update bogus folder if applicable
+    // We don't print anything manually, updating the filter will automatically trigger a redraw of all relevant stuff
-    if (isBogusFolder(existingTag)) {
-        // Update bogus drilldown
-        if ($(this).hasClass('selected')) {
-            appendTagToList($('.rm_tag_controls .rm_tag_bogus_drilldown'), existingTag, { removable: true });
-        } else {
-            $(listElement).closest('.rm_tag_controls').find(`.rm_tag_bogus_drilldown .tag[id=${tagId}]`).remove();
-        }
-    }
-
     runTagFilters(listElement);
-    updateTagFilterIndicator();
 }
 
 function toggleTagThreeState(element, { stateOverride = undefined, simulateClick = false } = {}) {
     const states = Object.keys(FILTER_STATES);
 
+    // Make it clear we're getting indexes and handling the 'not found' case in one place
+    function getStateIndex(key, fallback) {
+        const index = states.indexOf(key);
+        return index !== -1 ? index : states.indexOf(fallback);
+    }
+
     const overrideKey = states.includes(stateOverride) ? stateOverride : Object.keys(FILTER_STATES).find(key => FILTER_STATES[key] === stateOverride);
 
-    const currentStateIndex = states.indexOf(element.attr('data-toggle-state')) ?? states.length - 1;
+    const currentStateIndex = getStateIndex(element.attr('data-toggle-state'), DEFAULT_FILTER_STATE);
-    const targetStateIndex = overrideKey !== undefined ? states.indexOf(overrideKey) : (currentStateIndex + 1) % states.length;
+    const targetStateIndex = overrideKey !== undefined ? getStateIndex(overrideKey, DEFAULT_FILTER_STATE) : (currentStateIndex + 1) % states.length;
 
     if (simulateClick) {
         // Calculate how many clicks are needed to go from the current state to the target state
@@ -695,10 +687,8 @@ function runTagFilters(listElement) {
 }
 
 function printTagFilters(type = tag_filter_types.character) {
-    const filterData = structuredClone(entitiesFilter.getFilterData(FILTER_TYPES.TAG));
     const FILTER_SELECTOR = type === tag_filter_types.character ? CHARACTER_FILTER_SELECTOR : GROUP_FILTER_SELECTOR;
     $(FILTER_SELECTOR).empty();
-    $(FILTER_SELECTOR).siblings('.rm_tag_bogus_drilldown').empty();
 
     // Print all action tags. (Exclude folder if that setting isn't chosen)
     const actionTags = Object.values(ACTIONABLE_TAGS).filter(tag => power_user.bogus_folders || tag.id != ACTIONABLE_TAGS.FOLDER.id);
@@ -708,18 +698,21 @@ function printTagFilters(type = tag_filter_types.character) {
     printTagList($(FILTER_SELECTOR), { empty: false, tags: inListActionTags, tagActionSelector: tag => tag.action, tagOptions: { isGeneralList: true } });
 
     const characterTagIds = Object.values(tag_map).flat();
-    const tagsToDisplay = tags
+    const tagsToDisplay = tags.filter(x => characterTagIds.includes(x.id)).sort(compareTagsForSort);
-        .filter(x => characterTagIds.includes(x.id))
-        .sort(compareTagsForSort);
     printTagList($(FILTER_SELECTOR), { empty: false, tags: tagsToDisplay, tagOptions: { selectable: true, isGeneralList: true } });
 
-    runTagFilters(FILTER_SELECTOR);
+    // Print bogus folder navigation
+    const bogusDrilldown = $(FILTER_SELECTOR).siblings('.rm_tag_bogus_drilldown');
+    bogusDrilldown.empty();
+    if (power_user.bogus_folders && bogusDrilldown.length > 0) {
+        const filterData = structuredClone(entitiesFilter.getFilterData(FILTER_TYPES.TAG));
+        const navigatedTags = filterData.selected.map(x => tags.find(t => t.id == x)).filter(x => isBogusFolder(x));
 
-    // Simulate clicks on all "selected" tags when we reprint, otherwise their filter gets lost. "excluded" is persisted.
+        printTagList(bogusDrilldown, { tags: navigatedTags, tagOptions: { removable: true } });
-    for (const tagId of filterData.selected) {
-        toggleTagThreeState($(`${FILTER_SELECTOR} .tag[id="${tagId}"]`), { stateOverride: FILTER_STATES.SELECTED, simulateClick: true });
     }
 
+    runTagFilters(FILTER_SELECTOR);
+
     if (power_user.show_tag_filters) {
         $('.rm_tag_controls .showTagList').addClass('selected');
         $('.rm_tag_controls').find('.tag:not(.actionable)').show();