From 544072e40d55212ef4b1c4c82c084a0db31772f2 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Wed, 7 May 2025 10:14:36 +0200 Subject: [PATCH] Initial impl with reducer --- .../EditableConstraint.tsx | 212 +++++++++++------- .../EditableConstraintWrapper.tsx | 184 ++++++++------- .../LegalValuesSelector.tsx | 39 ++-- .../FeatureStrategyConstraints/ValueList.tsx | 5 +- 4 files changed, 251 insertions(+), 189 deletions(-) diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx index 6aead7374c..701b725dbd 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx @@ -4,22 +4,12 @@ import { useConstraintInput, type Input, } from 'component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/useConstraintInput/useConstraintInput'; -import { - DATE_AFTER, - dateOperators, - IN, - stringOperators, - type Operator, -} from 'constants/operators'; +import { stringOperators, type Operator } from 'constants/operators'; import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext'; import type { IUnleashContextDefinition } from 'interfaces/context'; import type { IConstraint } from 'interfaces/strategy'; -import { useEffect, useMemo, useRef, useState, type FC } from 'react'; -import { oneOf } from 'utils/oneOf'; -import { - CURRENT_TIME_CONTEXT_FIELD, - operatorsForContext, -} from 'utils/operatorsForContext'; +import { useMemo, useRef, type FC } from 'react'; +import { operatorsForContext } from 'utils/operatorsForContext'; import { ConstraintOperatorSelect } from './ConstraintOperatorSelect'; import { HtmlTooltip } from 'component/common/HtmlTooltip/HtmlTooltip'; import Delete from '@mui/icons-material/Delete'; @@ -136,6 +126,8 @@ const ButtonPlaceholder = styled('div')(({ theme }) => ({ // screens), but still retain necessary space for the button when it's all // on one line. width: theme.spacing(2), + // fontSize: theme.typography.body1.fontSize + fontSize: theme.fontSizes.extraLargeHeader, })); const StyledCaseInsensitiveIcon = styled(CaseInsensitiveIcon)(({ theme }) => ({ @@ -175,36 +167,48 @@ const getInputType = (input: Input): InputType => { } }; +type ConstraintUpdateAction = + | { type: 'add value(s)'; payload: string[] } + | { type: 'set value'; payload: string } + | { type: 'clear values' } + | { type: 'remove value from list'; payload: string } + | { type: 'set context field'; payload: string } + | { type: 'set operator'; payload: Operator } + | { type: 'toggle case sensitivity' } + | { type: 'toggle inverted operator' }; + type Props = { localConstraint: IConstraint; - setContextName: (contextName: string) => void; - setOperator: (operator: Operator) => void; - setLocalConstraint: React.Dispatch>; + // setContextName: (contextName: string) => void; + // setOperator: (operator: Operator) => void; + // setLocalConstraint: React.Dispatch>; onDelete?: () => void; - toggleInvertedOperator: () => void; - toggleCaseSensitivity: () => void; + // toggleInvertedOperator: () => void; + // toggleCaseSensitivity: () => void; contextDefinition: Pick; constraintValues: string[]; constraintValue: string; - setValue: (value: string) => void; - setValues: (values: string[]) => void; - removeValue: (index: number) => void; + // setValue: (value: string) => void; + // setValues: (values: string[]) => void; + // removeValue: (index: number) => void; + updateConstraint: (action: ConstraintUpdateAction) => void; }; export const EditableConstraint: FC = ({ localConstraint, - setLocalConstraint, - setContextName, - setOperator, + // setLocalConstraint, + // setContextName, + // setOperator, onDelete, - toggleInvertedOperator, - toggleCaseSensitivity, + // toggleInvertedOperator, + // toggleCaseSensitivity, contextDefinition, constraintValues, constraintValue, - setValue, - setValues, - removeValue, + // setValue, + // setValues, + // removeValue, + updateConstraint, }) => { const { input } = useConstraintInput({ contextDefinition, @@ -213,8 +217,7 @@ export const EditableConstraint: FC = ({ const { context } = useUnleashContext(); const { contextName, operator } = localConstraint; - const [showCaseSensitiveButton, setShowCaseSensitiveButton] = - useState(false); + const showCaseSensitiveButton = stringOperators.includes(operator); const deleteButtonRef = useRef(null); const addValuesButtonRef = useRef(null); const inputType = getInputType(input); @@ -224,29 +227,28 @@ export const EditableConstraint: FC = ({ this will check if the context field is the current time context field AND check if it is not already using one of the date operators (to not overwrite if there is existing data). */ - useEffect(() => { - if ( - contextName === CURRENT_TIME_CONTEXT_FIELD && - !oneOf(dateOperators, operator) - ) { - setLocalConstraint((prev) => ({ - ...prev, - operator: DATE_AFTER, - value: new Date().toISOString(), - })); - } else if ( - contextName !== CURRENT_TIME_CONTEXT_FIELD && - oneOf(dateOperators, operator) - ) { - setOperator(IN); - } - - if (oneOf(stringOperators, operator)) { - setShowCaseSensitiveButton(true); - } else { - setShowCaseSensitiveButton(false); - } - }, [contextName, setOperator, operator, setLocalConstraint]); + // useEffect(() => { + // if ( + // contextName === CURRENT_TIME_CONTEXT_FIELD && + // !oneOf(dateOperators, operator) + // ) { + // setLocalConstraint((prev) => ({ + // ...prev, + // operator: DATE_AFTER, + // value: new Date().toISOString(), + // })); + // } else if ( + // contextName !== CURRENT_TIME_CONTEXT_FIELD && + // oneOf(dateOperators, operator) + // ) { + // setOperator(IN); + // } + // if (oneOf(stringOperators, operator)) { + // setShowCaseSensitiveButton(true); + // } else { + // setShowCaseSensitiveButton(false); + // } + // }, [contextName, setOperator, operator, setLocalConstraint]); if (!context) { return null; @@ -257,21 +259,21 @@ export const EditableConstraint: FC = ({ }); const onOperatorChange = (operator: Operator) => { - if (oneOf(stringOperators, operator)) { - setShowCaseSensitiveButton(true); - } else { - setShowCaseSensitiveButton(false); - } - - if (oneOf(dateOperators, operator)) { - setLocalConstraint((prev) => ({ - ...prev, - operator: operator, - value: new Date().toISOString(), - })); - } else { - setOperator(operator); - } + updateConstraint({ type: 'set operator', payload: operator }); + // if (oneOf(stringOperators, operator)) { + // setShowCaseSensitiveButton(true); + // } else { + // setShowCaseSensitiveButton(false); + // } + // if (oneOf(dateOperators, operator)) { + // setLocalConstraint((prev) => ({ + // ...prev, + // operator: operator, + // value: new Date().toISOString(), + // })); + // } else { + // setOperator(operator); + // } }; const validator = useMemo(() => constraintValidator(input), [input]); @@ -280,7 +282,12 @@ export const EditableConstraint: FC = ({ case 'date': return ( + updateConstraint({ + type: 'set value', + payload: value, + }) + } value={localConstraint.value} validator={validator} /> @@ -290,9 +297,14 @@ export const EditableConstraint: FC = ({ { - setValue(newValue); + updateConstraint({ + type: 'set value', + payload: newValue, + }); }} - removeValue={() => setValue('')} + removeValue={() => + updateConstraint({ type: 'clear values' }) + } currentValue={localConstraint.value} helpText={ inputType.type === 'number' @@ -311,12 +323,10 @@ export const EditableConstraint: FC = ({ helpText='Maximum 100 char length per value' ref={addValuesButtonRef} onAddValues={(newValues) => { - // todo (`addEditStrategy`): move deduplication logic higher up in the context handling - const combinedValues = new Set([ - ...(localConstraint.values || []), - ...newValues, - ]); - setValues(Array.from(combinedValues)); + updateConstraint({ + type: 'add value(s)', + payload: newValues, + }); }} /> ); @@ -338,14 +348,23 @@ export const EditableConstraint: FC = ({ autoFocus options={constraintNameOptions} value={contextName || ''} - onChange={setContextName} + onChange={(contextField) => + updateConstraint({ + type: 'set context field', + payload: contextField, + }) + } variant='standard' /> + updateConstraint({ + type: 'toggle inverted operator', + }) + } > {localConstraint.inverted ? ( @@ -370,7 +389,11 @@ export const EditableConstraint: FC = ({ {showCaseSensitiveButton ? ( + updateConstraint({ + type: 'toggle case sensitivity', + }) + } > {localConstraint.caseInsensitive ? ( @@ -390,8 +413,12 @@ export const EditableConstraint: FC = ({ + updateConstraint({ + type: 'remove value from list', + payload: value, + }) + } getExternalFocusTarget={() => addValuesButtonRef.current ?? deleteButtonRef.current @@ -422,8 +449,23 @@ export const EditableConstraint: FC = ({ )} constraintValues={constraintValues} values={localConstraint.values || []} - setValuesWithRecord={setValues} - setValues={setValues} + clearAll={() => + updateConstraint({ + type: 'clear values', + }) + } + addValues={(newValues) => + updateConstraint({ + type: 'add value(s)', + payload: newValues, + }) + } + removeValue={(value) => + updateConstraint({ + type: 'remove value from list', + payload: value, + }) + } /> ) : null} diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx index 54547f33e9..b3ac5f12f5 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx @@ -1,10 +1,16 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useEffect, useState } from 'react'; import type { IConstraint } from 'interfaces/strategy'; import { cleanConstraint } from 'utils/cleanConstraint'; import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext'; import type { IUnleashContextDefinition } from 'interfaces/context'; -import type { Operator } from 'constants/operators'; +import { + DATE_AFTER, + dateOperators, + IN, + type Operator, +} from 'constants/operators'; import { EditableConstraint } from 'component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint'; +import { CURRENT_TIME_CONTEXT_FIELD } from 'utils/operatorsForContext'; interface IConstraintAccordionEditProps { constraint: IConstraint; @@ -34,15 +40,102 @@ const resolveContextDefinition = ( ); }; +type ConstraintUpdateAction = + | { type: 'add value(s)'; payload: string[] } + | { type: 'set value'; payload: string } + | { type: 'clear values' } + | { type: 'remove value from list'; payload: string } + | { type: 'set context field'; payload: string } + | { type: 'set operator'; payload: Operator } + | { type: 'toggle case sensitivity' } + | { type: 'toggle inverted operator' }; + export const EditableConstraintWrapper = ({ constraint, onDelete, onAutoSave, }: IConstraintAccordionEditProps) => { - const [localConstraint, setLocalConstraint] = useState( + const constraintReducer = ( + state: IConstraint, + action: ConstraintUpdateAction, + ): IConstraint => { + switch (action.type) { + case 'set context field': + if ( + action.payload === CURRENT_TIME_CONTEXT_FIELD && + !dateOperators.includes(state.operator) + ) { + return cleanConstraint({ + ...state, + operator: DATE_AFTER, + values: [], + value: new Date().toISOString(), + }); + } else if ( + action.payload !== CURRENT_TIME_CONTEXT_FIELD && + dateOperators.includes(state.operator) + ) { + return cleanConstraint({ + ...state, + operator: IN, + values: [], + value: '', + }); + } + + return cleanConstraint({ + ...state, + contextName: action.payload, + values: [], + value: '', + }); + case 'set operator': + return cleanConstraint({ + ...state, + operator: action.payload, + values: [], + value: '', + }); + case 'add value(s)': { + const combinedValues = new Set([ + ...(state.values || []), + ...action.payload, + ]); + return { ...state, values: Array.from(combinedValues) }; + } + case 'set value': + return { ...state, value: action.payload }; + case 'toggle inverted operator': + return { ...state, inverted: !state.inverted }; + case 'toggle case sensitivity': + return { ...state, caseInsensitive: !state.inverted }; + case 'remove value from list': + return { + ...state, + values: (state.values ?? []).filter( + (value) => value !== action.payload, + ), + }; + case 'clear values': + return cleanConstraint({ ...state, values: [], value: '' }); + } + }; + + // const [state, dispatch] = useReducer( + // constraintReducer, + // cleanConstraint(constraint), + // ); + + const [localConstraint, setLocalConstraint] = useState( cleanConstraint(constraint), ); + const updateConstraint = (action: ConstraintUpdateAction) => { + const nextState = constraintReducer(localConstraint, action); + setLocalConstraint(nextState); + onAutoSave(nextState); + }; + const { context } = useUnleashContext(); const [contextDefinition, setContextDefinition] = useState( resolveContextDefinition(context, localConstraint.contextName), @@ -54,97 +147,14 @@ export const EditableConstraintWrapper = ({ ); }, [localConstraint.contextName, context]); - const setContextName = useCallback((contextName: string) => { - setLocalConstraint((prev) => { - const localConstraint = cleanConstraint({ - ...prev, - contextName, - values: [], - value: '', - }); - - onAutoSave(localConstraint); - return localConstraint; - }); - }, []); - - const setOperator = useCallback((operator: Operator) => { - setLocalConstraint((prev) => { - const localConstraint = cleanConstraint({ - ...prev, - operator, - values: [], - value: '', - }); - - onAutoSave(localConstraint); - return localConstraint; - }); - }, []); - - const setValues = useCallback((values: string[]) => { - setLocalConstraint((prev) => { - const localConstraint = { ...prev, values }; - - onAutoSave(localConstraint); - return localConstraint; - }); - }, []); - - const setValue = useCallback((value: string) => { - setLocalConstraint((prev) => { - const localConstraint = { ...prev, value }; - - onAutoSave(localConstraint); - return localConstraint; - }); - }, []); - - const setInvertedOperator = () => { - setLocalConstraint((prev) => { - const localConstraint = { ...prev, inverted: !prev.inverted }; - - onAutoSave(localConstraint); - return localConstraint; - }); - }; - - const setCaseInsensitive = useCallback(() => { - setLocalConstraint((prev) => { - const localConstraint = { - ...prev, - caseInsensitive: !prev.caseInsensitive, - }; - - onAutoSave(localConstraint); - return localConstraint; - }); - }, []); - - const removeValue = useCallback( - (index: number) => { - const valueCopy = [...localConstraint.values!]; - valueCopy.splice(index, 1); - setValues(valueCopy); - }, - [localConstraint], - ); - return ( ); }; diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx index 13a9580743..2e5f2d07bb 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx @@ -15,8 +15,9 @@ interface IRestrictiveLegalValuesProps { }; constraintValues: string[]; values: string[]; - setValues: (values: string[]) => void; - setValuesWithRecord: (values: string[]) => void; + addValues: (values: string[]) => void; + removeValue: (values: string) => void; + clearAll: () => void; beforeValues?: JSX.Element; } @@ -72,8 +73,11 @@ const LegalValuesSelectorWidget = styled('article')(({ theme }) => ({ export const LegalValuesSelector = ({ data, values, - setValues, - setValuesWithRecord, + // setValues, + addValues, + removeValue, + clearAll, + // setValuesWithRecord, constraintValues, }: IRestrictiveLegalValuesProps) => { const [filter, setFilter] = useState(''); @@ -103,20 +107,23 @@ export const LegalValuesSelector = ({ useEffect(() => { if (illegalValues.length > 0) { - setValues(cleanDeletedLegalValues(values)); + console.log('would clean deleted values here'); + // setValues(cleanDeletedLegalValues(values)); } }, []); const onChange = (legalValue: string) => { if (valuesMap[legalValue]) { - const index = values.findIndex((value) => value === legalValue); - const newValues = [...values]; - newValues.splice(index, 1); - setValuesWithRecord(newValues); + removeValue(legalValue); + // const index = values.findIndex((value) => value === legalValue); + // const newValues = [...values]; + // newValues.splice(index, 1); + // setValuesWithRecord(newValues); return; } - setValuesWithRecord([...cleanDeletedLegalValues(values), legalValue]); + addValues([legalValue]); + // setValuesWithRecord([...cleanDeletedLegalValues(values), legalValue]); }; const isAllSelected = legalValues.every((value) => @@ -125,11 +132,15 @@ export const LegalValuesSelector = ({ const onSelectAll = () => { if (isAllSelected) { - return setValuesWithRecord([]); + clearAll(); + return; + // return setValuesWithRecord([]); + } else { + addValues(legalValues.map(({ value }) => value)); } - setValuesWithRecord([ - ...legalValues.map((legalValue) => legalValue.value), - ]); + // setValuesWithRecord([ + // ...legalValues.map((legalValue) => legalValue.value), + // ]); }; const handleSearchKeyDown = (event: React.KeyboardEvent) => { diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx index a2754810c7..1c542b8f52 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx @@ -55,8 +55,7 @@ export const ValueChip = styled( type Props = { values: string[] | undefined; - removeValue: (index: number) => void; - setValues: (values: string[]) => void; + removeValue: (value: string) => void; // the element that should receive focus when all value chips are deleted getExternalFocusTarget: () => HTMLElement | null; }; @@ -102,7 +101,7 @@ export const ValueList: FC> = ({ label={value} onDelete={() => { nextFocusTarget(index)?.focus(); - removeValue(index); + removeValue(value); }} />