1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-06-23 01:16:27 +02:00

feat: add undo (#5879)

This PR adds undo functionality so you can restore the state of your
constraint if you make a mistake. We also amend the autosave
functionality to only apply when values are changed and you have a valid
value. See demo:

https://www.loom.com/share/da704da8aee94ac18d4caae697426802
This commit is contained in:
Fredrik Strand Oseberg 2024-01-15 08:47:59 +01:00 committed by GitHub
parent 0ab42ab45c
commit f7b285d340
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 184 additions and 43 deletions

View File

@ -81,7 +81,6 @@ const StyledAccordionDetails = styled(AccordionDetails)(({ theme }) => ({
export const ConstraintAccordionEdit = ({ export const ConstraintAccordionEdit = ({
constraint, constraint,
compact, compact,
onCancel,
onSave, onSave,
onDelete, onDelete,
onAutoSave, onAutoSave,
@ -89,6 +88,9 @@ export const ConstraintAccordionEdit = ({
const [localConstraint, setLocalConstraint] = useState<IConstraint>( const [localConstraint, setLocalConstraint] = useState<IConstraint>(
cleanConstraint(constraint), cleanConstraint(constraint),
); );
const [constraintChanges, setConstraintChanges] = useState<IConstraint[]>([
cleanConstraint(constraint),
]);
const { context } = useUnleashContext(); const { context } = useUnleashContext();
const [contextDefinition, setContextDefinition] = useState( const [contextDefinition, setContextDefinition] = useState(
@ -98,40 +100,89 @@ export const ConstraintAccordionEdit = ({
const [expanded, setExpanded] = useState(false); const [expanded, setExpanded] = useState(false);
const [action, setAction] = useState(''); const [action, setAction] = useState('');
const { input, validator, setError, error } = useConstraintInput({
contextDefinition,
localConstraint,
});
useEffect(() => { useEffect(() => {
// Setting expanded to true on mount will cause the accordion // Setting expanded to true on mount will cause the accordion
// animation to take effect and transition the expanded accordion in // animation to take effect and transition the expanded accordion in
setExpanded(true); setExpanded(true);
}, []); }, []);
useEffect(() => {
if (onAutoSave) {
onAutoSave(localConstraint);
}
}, [JSON.stringify(localConstraint)]);
useEffect(() => { useEffect(() => {
setContextDefinition( setContextDefinition(
resolveContextDefinition(context, localConstraint.contextName), resolveContextDefinition(context, localConstraint.contextName),
); );
}, [localConstraint.contextName, context]); }, [localConstraint.contextName, context]);
useEffect(() => {
setError('');
}, [setError]);
const onUndo = () => {
if (constraintChanges.length < 2) return;
const previousChange = constraintChanges[constraintChanges.length - 2];
setLocalConstraint(previousChange);
setConstraintChanges((prev) => prev.slice(0, prev.length - 1));
};
const recordChange = (localConstraint: IConstraint) => {
setConstraintChanges((prev) => [...prev, localConstraint]);
if (
onAutoSave &&
localConstraint.values &&
localConstraint.values.length > 0
) {
onAutoSave(localConstraint);
}
if (onAutoSave && localConstraint.value) {
onAutoSave(localConstraint);
}
};
const setContextName = useCallback((contextName: string) => { const setContextName = useCallback((contextName: string) => {
setLocalConstraint((prev) => ({ setLocalConstraint((prev) => {
const localConstraint = cleanConstraint({
...prev, ...prev,
contextName, contextName,
values: [], values: [],
value: '', value: '',
})); });
recordChange(localConstraint);
return localConstraint;
});
}, []); }, []);
const setOperator = useCallback((operator: Operator) => { const setOperator = useCallback((operator: Operator) => {
setLocalConstraint((prev) => ({ setLocalConstraint((prev) => {
const localConstraint = cleanConstraint({
...prev, ...prev,
operator, operator,
values: [], values: [],
value: '', value: '',
})); });
recordChange(localConstraint);
return localConstraint;
});
}, []);
const setValuesWithRecord = useCallback((values: string[]) => {
setLocalConstraint((prev) => {
const localConstraint = { ...prev, values };
recordChange(localConstraint);
return localConstraint;
});
}, []); }, []);
const setValues = useCallback((values: string[]) => { const setValues = useCallback((values: string[]) => {
@ -143,18 +194,36 @@ export const ConstraintAccordionEdit = ({
}, []); }, []);
const setValue = useCallback((value: string) => { const setValue = useCallback((value: string) => {
setLocalConstraint((prev) => ({ ...prev, value })); setLocalConstraint((prev) => {
const localConstraint = { ...prev, value };
recordChange(localConstraint);
return localConstraint;
});
}, []); }, []);
const setInvertedOperator = () => { const setInvertedOperator = () => {
setLocalConstraint((prev) => ({ ...prev, inverted: !prev.inverted })); setLocalConstraint((prev) => {
const localConstraint = { ...prev, inverted: !prev.inverted };
recordChange(localConstraint);
return localConstraint;
});
}; };
const setCaseInsensitive = useCallback(() => { const setCaseInsensitive = useCallback(() => {
setLocalConstraint((prev) => ({ setLocalConstraint((prev) => {
const localConstraint = {
...prev, ...prev,
caseInsensitive: !prev.caseInsensitive, caseInsensitive: !prev.caseInsensitive,
})); };
recordChange(localConstraint);
return localConstraint;
});
}, []); }, []);
const removeValue = useCallback( const removeValue = useCallback(
@ -207,18 +276,6 @@ export const ConstraintAccordionEdit = ({
} }
}; };
const { input, validator, setError, error } = useConstraintInput({
contextDefinition,
localConstraint,
});
useEffect(() => {
setError('');
setLocalConstraint((localConstraint) =>
cleanConstraint(localConstraint),
);
}, [localConstraint.operator, localConstraint.contextName, setError]);
return ( return (
<StyledForm> <StyledForm>
<StyledAccordion <StyledAccordion
@ -243,6 +300,8 @@ export const ConstraintAccordionEdit = ({
setInvertedOperator={setInvertedOperator} setInvertedOperator={setInvertedOperator}
setCaseInsensitive={setCaseInsensitive} setCaseInsensitive={setCaseInsensitive}
onDelete={onDelete} onDelete={onDelete}
onUndo={onUndo}
constraintChanges={constraintChanges}
/> />
</StyledAccordionSummary> </StyledAccordionSummary>
@ -257,6 +316,7 @@ export const ConstraintAccordionEdit = ({
> >
<ResolveInput <ResolveInput
setValues={setValues} setValues={setValues}
setValuesWithRecord={setValuesWithRecord}
setValue={setValue} setValue={setValue}
setError={setError} setError={setError}
localConstraint={localConstraint} localConstraint={localConstraint}

View File

@ -26,6 +26,7 @@ interface IResolveInputProps {
constraintValue: string; constraintValue: string;
setValue: (value: string) => void; setValue: (value: string) => void;
setValues: (values: string[]) => void; setValues: (values: string[]) => void;
setValuesWithRecord: (values: string[]) => void;
setError: React.Dispatch<React.SetStateAction<string>>; setError: React.Dispatch<React.SetStateAction<string>>;
removeValue: (index: number) => void; removeValue: (index: number) => void;
input: Input; input: Input;
@ -66,6 +67,7 @@ export const ResolveInput = ({
localConstraint, localConstraint,
setValue, setValue,
setValues, setValues,
setValuesWithRecord,
setError, setError,
removeValue, removeValue,
error, error,
@ -83,6 +85,7 @@ export const ResolveInput = ({
)} )}
constraintValues={constraintValues} constraintValues={constraintValues}
values={localConstraint.values || []} values={localConstraint.values || []}
setValuesWithRecord={setValuesWithRecord}
setValues={setValues} setValues={setValues}
error={error} error={error}
setError={setError} setError={setError}
@ -143,7 +146,7 @@ export const ResolveInput = ({
<FreeTextInput <FreeTextInput
values={localConstraint.values || []} values={localConstraint.values || []}
removeValue={removeValue} removeValue={removeValue}
setValues={setValues} setValues={setValuesWithRecord}
error={error} error={error}
setError={setError} setError={setError}
/> />
@ -154,7 +157,7 @@ export const ResolveInput = ({
<FreeTextInput <FreeTextInput
values={localConstraint.values || []} values={localConstraint.values || []}
removeValue={removeValue} removeValue={removeValue}
setValues={setValues} setValues={setValuesWithRecord}
error={error} error={error}
setError={setError} setError={setError}
/> />

View File

@ -14,6 +14,7 @@ test('should show alert when you have illegal legal values', async () => {
constraintValues={fixedValues} constraintValues={fixedValues}
values={localValues} values={localValues}
setValues={() => {}} setValues={() => {}}
setValuesWithRecord={() => {}}
error={''} error={''}
setError={() => {}} setError={() => {}}
/>, />,
@ -43,6 +44,7 @@ test('Should remove illegal legal values from internal value state when mounting
constraintValues={fixedValues} constraintValues={fixedValues}
values={localValues} values={localValues}
setValues={setValues} setValues={setValues}
setValuesWithRecord={() => {}}
error={''} error={''}
setError={() => {}} setError={() => {}}
/>, />,

View File

@ -18,6 +18,7 @@ interface IRestrictiveLegalValuesProps {
constraintValues: string[]; constraintValues: string[];
values: string[]; values: string[];
setValues: (values: string[]) => void; setValues: (values: string[]) => void;
setValuesWithRecord: (values: string[]) => void;
beforeValues?: JSX.Element; beforeValues?: JSX.Element;
error: string; error: string;
setError: React.Dispatch<React.SetStateAction<string>>; setError: React.Dispatch<React.SetStateAction<string>>;
@ -53,6 +54,7 @@ export const RestrictiveLegalValues = ({
data, data,
values, values,
setValues, setValues,
setValuesWithRecord,
error, error,
setError, setError,
constraintValues, constraintValues,
@ -96,11 +98,11 @@ export const RestrictiveLegalValues = ({
const index = values.findIndex((value) => value === legalValue); const index = values.findIndex((value) => value === legalValue);
const newValues = [...values]; const newValues = [...values];
newValues.splice(index, 1); newValues.splice(index, 1);
setValues(newValues); setValuesWithRecord(newValues);
return; return;
} }
setValues([...cleanDeletedLegalValues(values), legalValue]); setValuesWithRecord([...cleanDeletedLegalValues(values), legalValue]);
}; };
return ( return (

View File

@ -36,6 +36,8 @@ interface IConstraintAccordionViewHeader {
onDelete?: () => void; onDelete?: () => void;
setInvertedOperator: () => void; setInvertedOperator: () => void;
setCaseInsensitive: () => void; setCaseInsensitive: () => void;
onUndo: () => void;
constraintChanges: IConstraint[];
} }
const StyledHeaderContainer = styled('div')(({ theme }) => ({ const StyledHeaderContainer = styled('div')(({ theme }) => ({
@ -90,11 +92,13 @@ const StyledHeaderText = styled('p')(({ theme }) => ({
export const ConstraintAccordionEditHeader = ({ export const ConstraintAccordionEditHeader = ({
compact, compact,
constraintChanges,
localConstraint, localConstraint,
setLocalConstraint, setLocalConstraint,
setContextName, setContextName,
setOperator, setOperator,
onDelete, onDelete,
onUndo,
setInvertedOperator, setInvertedOperator,
setCaseInsensitive, setCaseInsensitive,
}: IConstraintAccordionViewHeader) => { }: IConstraintAccordionViewHeader) => {
@ -221,7 +225,12 @@ export const ConstraintAccordionEditHeader = ({
</StyledHeaderText> </StyledHeaderText>
} }
/> />
<ConstraintAccordionHeaderActions onDelete={onDelete} disableEdit /> <ConstraintAccordionHeaderActions
onDelete={onDelete}
onUndo={onUndo}
constraintChanges={constraintChanges}
disableEdit
/>
</StyledHeaderContainer> </StyledHeaderContainer>
); );
}; };

View File

@ -1,11 +1,14 @@
import React from 'react'; import React from 'react';
import { IconButton, styled, Tooltip } from '@mui/material'; import { IconButton, styled, Tooltip } from '@mui/material';
import { Delete, Edit } from '@mui/icons-material'; import { Delete, Edit, Undo } from '@mui/icons-material';
import { ConditionallyRender } from '../../ConditionallyRender/ConditionallyRender'; import { ConditionallyRender } from '../../ConditionallyRender/ConditionallyRender';
import { IConstraint } from 'interfaces/strategy';
interface ConstraintAccordionHeaderActionsProps { interface ConstraintAccordionHeaderActionsProps {
onDelete?: () => void; onDelete?: () => void;
onEdit?: () => void; onEdit?: () => void;
onUndo?: () => void;
constraintChanges?: IConstraint[];
disableEdit?: boolean; disableEdit?: boolean;
disableDelete?: boolean; disableDelete?: boolean;
} }
@ -21,6 +24,8 @@ const StyledHeaderActions = styled('div')(({ theme }) => ({
export const ConstraintAccordionHeaderActions = ({ export const ConstraintAccordionHeaderActions = ({
onEdit, onEdit,
onDelete, onDelete,
onUndo,
constraintChanges = [],
disableDelete = false, disableDelete = false,
disableEdit = false, disableEdit = false,
}: ConstraintAccordionHeaderActionsProps) => { }: ConstraintAccordionHeaderActionsProps) => {
@ -38,6 +43,13 @@ export const ConstraintAccordionHeaderActions = ({
onDelete(); onDelete();
}); });
const onUndoClick =
onUndo &&
((event: React.SyntheticEvent) => {
event.stopPropagation();
onUndo();
});
return ( return (
<StyledHeaderActions> <StyledHeaderActions>
<ConditionallyRender <ConditionallyRender
@ -48,12 +60,28 @@ export const ConstraintAccordionHeaderActions = ({
type='button' type='button'
onClick={onEditClick} onClick={onEditClick}
disabled={disableEdit} disabled={disableEdit}
data-testid='EDIT_CONSTRAINT_BUTTON'
> >
<Edit /> <Edit />
</IconButton> </IconButton>
</Tooltip> </Tooltip>
} }
/> />
<ConditionallyRender
condition={Boolean(onUndoClick) && constraintChanges.length > 1}
show={
<Tooltip title='Undo last change' arrow>
<IconButton
type='button'
onClick={onUndoClick}
disabled={disableDelete}
data-testid='UNDO_CONSTRAINT_CHANGE_BUTTON'
>
<Undo />
</IconButton>
</Tooltip>
}
/>
<ConditionallyRender <ConditionallyRender
condition={Boolean(onDeleteClick) && !disableDelete} condition={Boolean(onDeleteClick) && !disableDelete}
show={ show={

View File

@ -277,6 +277,43 @@ describe('NewFeatureStrategyCreate', () => {
expect(screen.getByText(values[2])).toBeInTheDocument(); expect(screen.getByText(values[2])).toBeInTheDocument();
}); });
test('Should undo changes made to constraints', async () => {
setupComponent();
const titleEl = await screen.findByText('Gradual rollout');
expect(titleEl).toBeInTheDocument();
const targetingEl = screen.getByText('Targeting');
fireEvent.click(targetingEl);
const addConstraintEl = await screen.findByText('Add constraint');
fireEvent.click(addConstraintEl);
const inputEl = screen.getByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputEl, {
target: { value: '6, 7, 8' },
});
const addBtn = await screen.findByText('Add values');
addBtn.click();
expect(screen.queryByText('6')).toBeInTheDocument();
expect(screen.queryByText('7')).toBeInTheDocument();
expect(screen.queryByText('8')).toBeInTheDocument();
const undoBtn = await screen.findByTestId(
'UNDO_CONSTRAINT_CHANGE_BUTTON',
);
undoBtn.click();
expect(screen.queryByText('6')).not.toBeInTheDocument();
expect(screen.queryByText('7')).not.toBeInTheDocument();
expect(screen.queryByText('8')).not.toBeInTheDocument();
});
test('Should remove constraint when no valid values are set and moving between tabs', async () => { test('Should remove constraint when no valid values are set and moving between tabs', async () => {
setupComponent(); setupComponent();