From c47aafff668288a07c22ef82261cfb8fc6fc17d6 Mon Sep 17 00:00:00 2001 From: WithoutPants <53250216+WithoutPants@users.noreply.github.com> Date: Mon, 12 Aug 2024 14:10:10 +1000 Subject: [PATCH] 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 --- .../src/components/List/FilterProvider.tsx | 2 +- ui/v2.5/src/components/List/ListFilter.tsx | 146 +++++++++--------- ui/v2.5/src/components/List/styles.scss | 4 + ui/v2.5/src/components/List/util.ts | 13 +- .../src/components/Shared/ClearableInput.tsx | 5 +- ui/v2.5/src/components/Shared/styles.scss | 2 + .../models/list-filter/criteria/criterion.ts | 80 ++++------ .../src/models/list-filter/criteria/gender.ts | 4 +- .../models/list-filter/criteria/performers.ts | 11 +- .../src/models/list-filter/criteria/phash.ts | 7 +- .../src/models/list-filter/criteria/rating.ts | 7 +- .../models/list-filter/criteria/stash-ids.ts | 7 +- 12 files changed, 134 insertions(+), 154 deletions(-) diff --git a/ui/v2.5/src/components/List/FilterProvider.tsx b/ui/v2.5/src/components/List/FilterProvider.tsx index 9f0abc4e0..1027de102 100644 --- a/ui/v2.5/src/components/List/FilterProvider.tsx +++ b/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 ( diff --git a/ui/v2.5/src/components/List/ListFilter.tsx b/ui/v2.5/src/components/List/ListFilter.tsx index 23244cbf0..bff14336c 100644 --- a/ui/v2.5/src/components/List/ListFilter.tsx +++ b/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 ( + + ); +}; interface IListFilterProps { onFilterUpdate: (newFilter: ListFilterModel) => void; @@ -48,44 +118,17 @@ export const ListFilter: React.FC = ({ 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 = ({ } }, [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 = ({ onFilterUpdate(newFilter); } - function onChangeQuery(event: React.FormEvent) { - 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 = ({ return ( <> -
-
- - -
+
+
diff --git a/ui/v2.5/src/components/List/styles.scss b/ui/v2.5/src/components/List/styles.scss index eeaa8527a..edfb9d2a7 100644 --- a/ui/v2.5/src/components/List/styles.scss +++ b/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; +} diff --git a/ui/v2.5/src/components/List/util.ts b/ui/v2.5/src/components/List/util.ts index 0cda1b4cb..69d3528bd 100644 --- a/ui/v2.5/src/components/List/util.ts +++ b/ui/v2.5/src/components/List/util.ts @@ -13,10 +13,10 @@ export function useFilterURL( setFilter: React.Dispatch>, 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 }; } diff --git a/ui/v2.5/src/components/Shared/ClearableInput.tsx b/ui/v2.5/src/components/Shared/ClearableInput.tsx index a9be946d8..28508e7ba 100644 --- a/ui/v2.5/src/components/Shared/ClearableInput.tsx +++ b/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; @@ -13,6 +15,7 @@ interface IClearableInput { } export const ClearableInput: React.FC = ({ + className, value, setValue, focus, @@ -37,7 +40,7 @@ export const ClearableInput: React.FC = ({ } return ( -
+
{ this.value = value; } - public abstract clone(): Criterion; + 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 { super(type, value); } - public clone(): Criterion { - 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 { - 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 { 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 { } } -export class MultiStringCriterion extends Criterion { +export abstract class MultiStringCriterion extends Criterion { constructor(type: CriterionOption, value: string[] = []) { super(type, value); } - public clone(): Criterion { - 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 { 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 { 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 { 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 { 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() { diff --git a/ui/v2.5/src/models/list-filter/criteria/gender.ts b/ui/v2.5/src/models/list-filter/criteria/gender.ts index 27e7bf170..31e5a38ac 100644 --- a/ui/v2.5/src/models/list-filter/criteria/gender.ts +++ b/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 { diff --git a/ui/v2.5/src/models/list-filter/criteria/performers.ts b/ui/v2.5/src/models/list-filter/criteria/performers.ts index a4ce1a1bf..e5d8178e0 100644 --- a/ui/v2.5/src/models/list-filter/criteria/performers.ts +++ b/ui/v2.5/src/models/list-filter/criteria/performers.ts @@ -33,11 +33,12 @@ export class PerformersCriterion extends Criterion { 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 { diff --git a/ui/v2.5/src/models/list-filter/criteria/phash.ts b/ui/v2.5/src/models/list-filter/criteria/phash.ts index ea794c30e..e1119bb65 100644 --- a/ui/v2.5/src/models/list-filter/criteria/phash.ts +++ b/ui/v2.5/src/models/list-filter/criteria/phash.ts @@ -29,11 +29,8 @@ export class PhashCriterion extends Criterion { 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() { diff --git a/ui/v2.5/src/models/list-filter/criteria/rating.ts b/ui/v2.5/src/models/list-filter/criteria/rating.ts index 59760a53d..ef18a7f2b 100644 --- a/ui/v2.5/src/models/list-filter/criteria/rating.ts +++ b/ui/v2.5/src/models/list-filter/criteria/rating.ts @@ -45,11 +45,8 @@ export class RatingCriterion extends Criterion { 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 { diff --git a/ui/v2.5/src/models/list-filter/criteria/stash-ids.ts b/ui/v2.5/src/models/list-filter/criteria/stash-ids.ts index 0ea7b0655..94d53ed92 100644 --- a/ui/v2.5/src/models/list-filter/criteria/stash-ids.ts +++ b/ui/v2.5/src/models/list-filter/criteria/stash-ids.ts @@ -27,11 +27,8 @@ export class StashIDCriterion extends Criterion { }); } - 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 {