Filter issue fixes (#5126)

* Fix filter reading from URL when not active
* Use alternative clone mechanism. Fixes weird filter hook behaviour
* Separate search term input component

ui/v2.5/src/components/List/FilterProvider.tsx

@@ -65,7 +65,7 @@ export const SetFilterURL = (props: {
 
   const { setFilter } = useFilterURL(filter, setFilterOrig, {
     defaultFilter,
-    setURL,
+    active: setURL,
   });
 
   return (

ui/v2.5/src/components/List/ListFilter.tsx

@@ -1,6 +1,5 @@
 import cloneDeep from "lodash-es/cloneDeep";
 import React, { useCallback, useEffect, useRef, useState } from "react";
-import cx from "classnames";
 import Mousetrap from "mousetrap";
 import { SortDirectionEnum } from "src/core/generated-graphql";
 import {
@@ -11,7 +10,6 @@ import {
   OverlayTrigger,
   Tooltip,
   InputGroup,
-  FormControl,
   Popover,
   Overlay,
 } from "react-bootstrap";
@@ -26,11 +24,83 @@ import {
   faCaretUp,
   faCheck,
   faRandom,
-  faTimes,
 } from "@fortawesome/free-solid-svg-icons";
 import { FilterButton } from "./Filters/FilterButton";
 import { useDebounce } from "src/hooks/debounce";
 import { View } from "./views";
+import { ClearableInput } from "../Shared/ClearableInput";
+
+export function useDebouncedSearchInput(
+  filter: ListFilterModel,
+  setFilter: (filter: ListFilterModel) => void
+) {
+  const callback = useCallback(
+    (value: string) => {
+      const newFilter = filter.clone();
+      newFilter.searchTerm = value;
+      newFilter.currentPage = 1;
+      setFilter(newFilter);
+    },
+    [filter, setFilter]
+  );
+
+  const onClear = useCallback(() => callback(""), [callback]);
+
+  const searchCallback = useDebounce(callback, 500);
+
+  return { searchCallback, onClear };
+}
+
+export const SearchTermInput: React.FC<{
+  filter: ListFilterModel;
+  onFilterUpdate: (newFilter: ListFilterModel) => void;
+}> = ({ filter, onFilterUpdate }) => {
+  const intl = useIntl();
+  const [localInput, setLocalInput] = useState(filter.searchTerm);
+
+  const focus = useFocus();
+  const [, setQueryFocus] = focus;
+
+  useEffect(() => {
+    setLocalInput(filter.searchTerm);
+  }, [filter.searchTerm]);
+
+  const { searchCallback, onClear } = useDebouncedSearchInput(
+    filter,
+    onFilterUpdate
+  );
+
+  useEffect(() => {
+    Mousetrap.bind("/", (e) => {
+      setQueryFocus();
+      e.preventDefault();
+    });
+
+    return () => {
+      Mousetrap.unbind("/");
+    };
+  });
+
+  function onSetQuery(value: string) {
+    setLocalInput(value);
+
+    if (!value) {
+      onClear();
+    }
+
+    searchCallback(value);
+  }
+
+  return (
+    <ClearableInput
+      className="search-term-input"
+      focus={focus}
+      value={localInput}
+      setValue={onSetQuery}
+      placeholder={`${intl.formatMessage({ id: "actions.search" })}…`}
+    />
+  );
+};
 
 interface IListFilterProps {
   onFilterUpdate: (newFilter: ListFilterModel) => void;
@@ -48,44 +118,17 @@ export const ListFilter: React.FC<IListFilterProps> = ({
   view,
 }) => {
   const [customPageSizeShowing, setCustomPageSizeShowing] = useState(false);
-  const [queryRef, setQueryFocus] = useFocus();
-  const [queryClearShowing, setQueryClearShowing] = useState(
-    !!filter.searchTerm
-  );
   const perPageSelect = useRef(null);
   const [perPageInput, perPageFocus] = useFocus();
 
   const filterOptions = filter.options;
 
-  const searchQueryUpdated = useCallback(
-    (value: string) => {
-      const newFilter = cloneDeep(filter);
-      newFilter.searchTerm = value;
-      newFilter.currentPage = 1;
-      onFilterUpdate(newFilter);
-    },
-    [filter, onFilterUpdate]
-  );
-
-  const searchCallback = useDebounce((value: string) => {
-    const newFilter = cloneDeep(filter);
-    newFilter.searchTerm = value;
-    newFilter.currentPage = 1;
-    onFilterUpdate(newFilter);
-  }, 500);
-
   const intl = useIntl();
 
   useEffect(() => {
-    Mousetrap.bind("/", (e) => {
-      setQueryFocus();
-      e.preventDefault();
-    });
-
     Mousetrap.bind("r", () => onReshuffleRandomSort());
 
     return () => {
-      Mousetrap.unbind("/");
       Mousetrap.unbind("r");
     };
   });
@@ -96,14 +139,6 @@ export const ListFilter: React.FC<IListFilterProps> = ({
     }
   }, [customPageSizeShowing, perPageFocus]);
 
-  // clear search input when filter is cleared
-  useEffect(() => {
-    if (!filter.searchTerm) {
-      if (queryRef.current) queryRef.current.value = "";
-      setQueryClearShowing(false);
-    }
-  }, [filter.searchTerm, queryRef]);
-
   function onChangePageSize(val: string) {
     if (val === "custom") {
       // added timeout since Firefox seems to trigger the rootClose immediately
@@ -125,18 +160,6 @@ export const ListFilter: React.FC<IListFilterProps> = ({
     onFilterUpdate(newFilter);
   }
 
-  function onChangeQuery(event: React.FormEvent<HTMLInputElement>) {
-    searchCallback(event.currentTarget.value);
-    setQueryClearShowing(!!event.currentTarget.value);
-  }
-
-  function onClearQuery() {
-    if (queryRef.current) queryRef.current.value = "";
-    searchQueryUpdated("");
-    setQueryFocus();
-    setQueryClearShowing(false);
-  }
-
   function onChangeSortDirection() {
     const newFilter = cloneDeep(filter);
     if (filter.sortDirection === SortDirectionEnum.Asc) {
@@ -209,27 +232,8 @@ export const ListFilter: React.FC<IListFilterProps> = ({
 
     return (
       <>
-        <div className="mb-2 mr-2 d-flex">
-          <div className="flex-grow-1 query-text-field-group">
-            <FormControl
-              ref={queryRef}
-              placeholder={`${intl.formatMessage({ id: "actions.search" })}…`}
-              defaultValue={filter.searchTerm}
-              onInput={onChangeQuery}
-              className="query-text-field bg-secondary text-white border-secondary"
-            />
-            <Button
-              variant="secondary"
-              onClick={onClearQuery}
-              title={intl.formatMessage({ id: "actions.clear" })}
-              className={cx(
-                "query-text-field-clear",
-                queryClearShowing ? "" : "d-none"
-              )}
-            >
-              <Icon icon={faTimes} />
-            </Button>
-          </div>
+        <div className="mb-2 d-flex">
+          <SearchTermInput filter={filter} onFilterUpdate={onFilterUpdate} />
         </div>
 
         <ButtonGroup className="mr-2 mb-2">

ui/v2.5/src/components/List/styles.scss

@@ -571,3 +571,7 @@ input[type="range"].zoom-slider {
     border-left: none;
   }
 }
+
+.search-term-input {
+  margin-right: 0.5rem;
+}

ui/v2.5/src/components/List/util.ts

@@ -13,10 +13,10 @@ export function useFilterURL(
   setFilter: React.Dispatch<React.SetStateAction<ListFilterModel>>,
   options?: {
     defaultFilter?: ListFilterModel;
-    setURL?: boolean;
+    active?: boolean;
   }
 ) {
-  const { defaultFilter, setURL = true } = options ?? {};
+  const { defaultFilter, active = true } = options ?? {};
 
   const history = useHistory();
   const location = useLocation();
@@ -28,7 +28,7 @@ export function useFilterURL(
     ) => {
       const newFilter = isFunction(value) ? value(filter) : value;
 
-      if (setURL) {
+      if (active) {
         const newParams = newFilter.makeQueryParameters();
         history.replace({ ...history.location, search: newParams });
       } else {
@@ -36,12 +36,15 @@ export function useFilterURL(
         setFilter(newFilter);
       }
     },
-    [history, setURL, setFilter, filter]
+    [history, active, setFilter, filter]
   );
 
   // This hook runs on every page location change (ie navigation),
   // and updates the filter accordingly.
   useEffect(() => {
+    // don't apply if active is false
+    if (!active) return;
+
     // re-init to load default filter on empty new query params
     if (!location.search) {
       if (defaultFilter) updateFilter(defaultFilter.clone());
@@ -58,7 +61,7 @@ export function useFilterURL(
         return prevFilter;
       }
     });
-  }, [location.search, defaultFilter, setFilter, updateFilter]);
+  }, [active, location.search, defaultFilter, setFilter, updateFilter]);
 
   return { setFilter: updateFilter };
 }

ui/v2.5/src/components/Shared/ClearableInput.tsx

@@ -4,8 +4,10 @@ import { faTimes } from "@fortawesome/free-solid-svg-icons";
 import { useIntl } from "react-intl";
 import { Icon } from "./Icon";
 import useFocus from "src/utils/focus";
+import cx from "classnames";
 
 interface IClearableInput {
+  className?: string;
   value: string;
   setValue: (value: string) => void;
   focus?: ReturnType<typeof useFocus>;
@@ -13,6 +15,7 @@ interface IClearableInput {
 }
 
 export const ClearableInput: React.FC<IClearableInput> = ({
+  className,
   value,
   setValue,
   focus,
@@ -37,7 +40,7 @@ export const ClearableInput: React.FC<IClearableInput> = ({
   }
 
   return (
-    <div className="clearable-input-group">
+    <div className={cx("clearable-input-group", className)}>
       <FormControl
         ref={queryRef}
         placeholder={placeholder}

ui/v2.5/src/components/Shared/styles.scss

@@ -492,12 +492,14 @@ div.react-datepicker {
 
 .clearable-text-field-clear {
   background-color: $secondary;
+  bottom: 0;
   color: $muted-gray;
   font-size: 0.875rem;
   margin: 0.375rem 0.75rem;
   padding: 0;
   position: absolute;
   right: 0;
+  top: 0;
   z-index: 4;
 
   &:hover,

ui/v2.5/src/models/list-filter/criteria/criterion.ts

@@ -89,7 +89,13 @@ export abstract class Criterion<V extends CriterionValue> {
     this.value = value;
   }
 
-  public abstract clone(): Criterion<V>;
+  public clone() {
+    const ret = Object.assign(Object.create(Object.getPrototypeOf(this)), this);
+    ret.cloneValues();
+    return ret;
+  }
+
+  protected cloneValues() {}
 
   public static getModifierLabel(intl: IntlShape, modifier: CriterionModifier) {
     const modifierMessageID = modifierMessageIDs[modifier];
@@ -257,13 +263,8 @@ export class ILabeledIdCriterion extends Criterion<ILabeledId[]> {
     super(type, value);
   }
 
-  public clone(): Criterion<ILabeledId[]> {
-    const newCriterion = new ILabeledIdCriterion(
-      this.criterionOption,
-      this.value.map((v) => ({ ...v }))
-    );
-    newCriterion.modifier = this.modifier;
-    return newCriterion;
+  public cloneValues() {
+    this.value = this.value.map((v) => ({ ...v }));
   }
 
   protected getLabelValue(_intl: IntlShape): string {
@@ -301,17 +302,12 @@ export class IHierarchicalLabeledIdCriterion extends Criterion<IHierarchicalLabe
     super(type, value);
   }
 
-  public clone(): Criterion<IHierarchicalLabelValue> {
-    const newCriterion = new IHierarchicalLabeledIdCriterion(
-      this.criterionOption,
-      {
-        ...this.value,
-        items: this.value.items.map((v) => ({ ...v })),
-        excluded: this.value.excluded.map((v) => ({ ...v })),
-      }
-    );
-    newCriterion.modifier = this.modifier;
-    return newCriterion;
+  public cloneValues() {
+    this.value = {
+      ...this.value,
+      items: this.value.items.map((v) => ({ ...v })),
+      excluded: this.value.excluded.map((v) => ({ ...v })),
+    };
   }
 
   override get modifier(): CriterionModifier {
@@ -512,13 +508,6 @@ export class StringCriterion extends Criterion<string> {
     super(type, "");
   }
 
-  public clone() {
-    const newCriterion = new StringCriterion(this.criterionOption);
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = this.value;
-    return newCriterion;
-  }
-
   protected getLabelValue(_intl: IntlShape) {
     return this.value;
   }
@@ -532,18 +521,13 @@ export class StringCriterion extends Criterion<string> {
   }
 }
 
-export class MultiStringCriterion extends Criterion<string[]> {
+export abstract class MultiStringCriterion extends Criterion<string[]> {
   constructor(type: CriterionOption, value: string[] = []) {
     super(type, value);
   }
 
-  public clone(): Criterion<string[]> {
-    const newCriterion = new MultiStringCriterion(
-      this.criterionOption,
-      this.value.slice()
-    );
-    newCriterion.modifier = this.modifier;
-    return newCriterion;
+  public cloneValues() {
+    this.value = this.value.slice();
   }
 
   protected getLabelValue(_intl: IntlShape) {
@@ -718,11 +702,8 @@ export class NumberCriterion extends Criterion<INumberValue> {
     super(type, { value: undefined, value2: undefined });
   }
 
-  public clone() {
-    const newCriterion = new NumberCriterion(this.criterionOption);
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   public get value(): INumberValue {
@@ -803,11 +784,8 @@ export class DurationCriterion extends Criterion<INumberValue> {
     super(type, { value: undefined, value2: undefined });
   }
 
-  public clone() {
-    const newCriterion = new DurationCriterion(this.criterionOption);
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   public toCriterionInput(): IntCriterionInput {
@@ -887,11 +865,8 @@ export class DateCriterion extends Criterion<IDateValue> {
     super(type, { value: "", value2: undefined });
   }
 
-  public clone() {
-    const newCriterion = new DateCriterion(this.criterionOption);
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   public encodeValue() {
@@ -993,11 +968,8 @@ export class TimestampCriterion extends Criterion<ITimestampValue> {
     super(type, { value: "", value2: undefined });
   }
 
-  public clone() {
-    const newCriterion = new TimestampCriterion(this.criterionOption);
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   public encodeValue() {

ui/v2.5/src/models/list-filter/criteria/gender.ts

@@ -25,8 +25,8 @@ export const GenderCriterionOption = new CriterionOption({
 });
 
 export class GenderCriterion extends MultiStringCriterion {
-  constructor() {
-    super(GenderCriterionOption);
+  constructor(value: string[] = []) {
+    super(GenderCriterionOption, value);
   }
 
   public toCriterionInput(): GenderCriterionInput {

ui/v2.5/src/models/list-filter/criteria/performers.ts

@@ -33,11 +33,12 @@ export class PerformersCriterion extends Criterion<ILabeledValueListValue> {
     super(PerformersCriterionOption, { items: [], excluded: [] });
   }
 
-  public clone() {
-    const newCriterion = new PerformersCriterion();
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = {
+      ...this.value,
+      items: this.value.items.map((v) => ({ ...v })),
+      excluded: this.value.excluded.map((v) => ({ ...v })),
+    };
   }
 
   override get modifier(): CriterionModifier {

ui/v2.5/src/models/list-filter/criteria/phash.ts

@@ -29,11 +29,8 @@ export class PhashCriterion extends Criterion<IPhashDistanceValue> {
     super(PhashCriterionOption, { value: "", distance: 0 });
   }
 
-  public clone() {
-    const newCriterion = new PhashCriterion();
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   protected getLabelValue() {

ui/v2.5/src/models/list-filter/criteria/rating.ts

@@ -45,11 +45,8 @@ export class RatingCriterion extends Criterion<INumberValue> {
     this.ratingSystem = ratingSystem;
   }
 
-  public clone() {
-    const newCriterion = new RatingCriterion(this.ratingSystem);
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   public get value(): INumberValue {

ui/v2.5/src/models/list-filter/criteria/stash-ids.ts

@@ -27,11 +27,8 @@ export class StashIDCriterion extends Criterion<IStashIDValue> {
     });
   }
 
-  public clone() {
-    const newCriterion = new StashIDCriterion();
-    newCriterion.modifier = this.modifier;
-    newCriterion.value = { ...this.value };
-    return newCriterion;
+  public cloneValues() {
+    this.value = { ...this.value };
   }
 
   public get value(): IStashIDValue {