Skip to content

Commit 16d4c9b

Browse files
shuktika12163Shuktika Jainrllyy97
authored
fix(designer): Fixed various bugs in undo/redo (#5899)
* Fix parameter panel not re-rendering - corner case * Fix delete issue with scope nodes * Add updateParameterConditionalVisibility to state save * fix type error * Fix floating action menu bugs - state per keystroke, add action, no undo for outputs * Fix card title state save on no changes and title update on blur * update tests * add more tests * revert unintended strings.json change * fix check for title update * revert strings.json change again --------- Co-authored-by: Shuktika Jain <[email protected]> Co-authored-by: Riley Evans <[email protected]>
1 parent dd68c2d commit 16d4c9b

File tree

19 files changed

+189
-56
lines changed

19 files changed

+189
-56
lines changed

‎libs/designer-ui/src/lib/dynamicallyaddedparameter/__test__/dynamicallyaddedparameter.spec.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ describe('ui/dynamicallyaddedparameter', () => {
7777
const { getByPlaceholderText } = render(<DynamicallyAddedParameter {...minimal} onTitleChange={onTitleChange} />);
7878
const newText = 'New text';
7979
fireEvent.change(getByPlaceholderText(minimal.titlePlaceholder!), { target: { value: newText } });
80+
expect(onTitleChange).not.toHaveBeenCalled();
81+
fireEvent.blur(getByPlaceholderText(minimal.titlePlaceholder!));
8082
expect(onTitleChange).toHaveBeenCalledWith(minimal.schemaKey, newText);
8183
});
8284

@@ -88,6 +90,8 @@ describe('ui/dynamicallyaddedparameter', () => {
8890
);
8991
const newText = 'New text';
9092
fireEvent.change(getByPlaceholderText(minimal.descriptionPlaceholder!), { target: { value: newText } });
93+
expect(onDescriptionChange).not.toHaveBeenCalled();
94+
fireEvent.blur(getByPlaceholderText(minimal.descriptionPlaceholder!));
9195
expect(onDescriptionChange).toHaveBeenCalledWith(minimal.schemaKey, newText);
9296
});
9397
});

‎libs/designer-ui/src/lib/dynamicallyaddedparameter/index.tsx

+23-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { IContextualMenuProps } from '@fluentui/react';
22
import { DirectionalHint, IconButton, TextField, TooltipHost } from '@fluentui/react';
33
import type React from 'react';
4+
import { useState } from 'react';
45
import { useIntl } from 'react-intl';
56
import StringStack from './plugins/stringstack';
67

@@ -230,6 +231,9 @@ export const DynamicallyAddedParameter = (props: DynamicallyAddedParameterProps)
230231
: dropdownTitleText
231232
: '';
232233

234+
const [titleValue, setTitleValue] = useState(title ?? '');
235+
const [descriptionValue, setDescriptionValue] = useState(props?.description ?? '');
236+
233237
const renderDynamicParameterContainer = (): JSX.Element => {
234238
const iconStyle = {
235239
background: `url('${icon}') no-repeat center`,
@@ -238,20 +242,34 @@ export const DynamicallyAddedParameter = (props: DynamicallyAddedParameterProps)
238242

239243
const onTitleChange = (e: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>, newValue?: string): void => {
240244
e.preventDefault();
241-
props.onTitleChange(schemaKey, newValue);
245+
setTitleValue(newValue ?? '');
242246
};
243247

244248
const onDescriptionChange = (e: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>, newValue?: string): void => {
245249
e.preventDefault();
246-
props?.onDescriptionChange?.(schemaKey, newValue);
250+
setDescriptionValue(newValue ?? '');
251+
};
252+
253+
const onTitleBlur = (): void => {
254+
props.onTitleChange(schemaKey, titleValue);
255+
};
256+
257+
const onDescriptionBlur = (): void => {
258+
props?.onDescriptionChange?.(schemaKey, descriptionValue);
247259
};
248260

249261
return (
250262
<>
251263
<div className="msla-dynamic-added-param-header">
252264
<div className="msla-dynamic-added-param-icon" style={iconStyle} />
253265
<div className="msla-dynamic-added-param-inputs-container">
254-
<TextField className="msla-dynamic-added-param-title" placeholder={titlePlaceholder} value={title} onChange={onTitleChange} />
266+
<TextField
267+
className="msla-dynamic-added-param-title"
268+
placeholder={titlePlaceholder}
269+
value={titleValue}
270+
onChange={onTitleChange}
271+
onBlur={onTitleBlur}
272+
/>
255273
<div className="msla-dynamic-added-param-value">{onRenderValueField(schemaKey)}</div>
256274
</div>
257275
<div className="msla-dynamic-add-param-menu-container">{renderMenuButton()}</div>
@@ -261,8 +279,9 @@ export const DynamicallyAddedParameter = (props: DynamicallyAddedParameterProps)
261279
<TextField
262280
className="msla-dynamic-added-param-description"
263281
placeholder={props?.descriptionPlaceholder}
264-
value={props?.description}
282+
value={descriptionValue}
265283
onChange={onDescriptionChange}
284+
onBlur={onDescriptionBlur}
266285
/>
267286
</div>
268287
)}

‎libs/designer-ui/src/lib/editor/base/index.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export type GetTokenPickerHandler = (
4444
tokenClickedCallback?: (token: ValueSegment) => void
4545
) => JSX.Element;
4646

47-
export type ChangeHandler = (newState: ChangeState) => void;
47+
export type ChangeHandler = (newState: ChangeState, skipStateSave?: boolean) => void;
4848
export type CallbackHandler = () => void;
4949
export type CastHandler = (value: ValueSegment[], type?: string, format?: string, suppressCasting?: boolean) => string;
5050

‎libs/designer-ui/src/lib/floatingactionmenu/floatingactionmenuinputs/index.tsx

+39-10
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useState } from 'react';
12
import constants from '../../constants';
23
import type { DynamicallyAddedParameterProps, DynamicallyAddedParameterTypeType } from '../../dynamicallyaddedparameter';
34
import { DynamicallyAddedParameter } from '../../dynamicallyaddedparameter';
@@ -80,7 +81,8 @@ export const FloatingActionMenuInputs = (props: FloatingActionMenuInputsProps):
8081
const { onChange } = props;
8182
if (onChange) {
8283
const value = getEmptySchemaValueSegmentForInitialization(!!props.useStaticInputs, props.isRequestApiConnectionTrigger);
83-
onChange({ value });
84+
// Update node parameters in root state but skip saving state for undo/redo since add action already saved a state
85+
onChange({ value }, /* skipStateSave */ true);
8486
}
8587
}
8688

@@ -209,15 +211,13 @@ export const FloatingActionMenuInputs = (props: FloatingActionMenuInputsProps):
209211
};
210212

211213
const onRenderValueField = (schemaKey: string) => {
212-
const onDescriptionChange = (e: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>, newPropertyValue?: string) => {
213-
e.preventDefault();
214-
onDynamicallyAddedParameterChange(schemaKey, 'description', newPropertyValue);
215-
};
216-
217-
const modelIndex = dynamicParameterModels.findIndex((model) => model.schemaKey === schemaKey);
218-
const description = dynamicParameterModels[modelIndex].properties.description;
219-
220-
return <TextField className="msla-dynamic-added-param-value-TextField" value={description} onChange={onDescriptionChange} />;
214+
return (
215+
<ValueField
216+
schemaKey={schemaKey}
217+
onDynamicallyAddedParameterChange={onDynamicallyAddedParameterChange}
218+
dynamicParameterModels={dynamicParameterModels}
219+
/>
220+
);
221221
};
222222

223223
const stringListValues = (schemaKey: string): string[] => {
@@ -338,3 +338,32 @@ export const FloatingActionMenuInputs = (props: FloatingActionMenuInputsProps):
338338
</FloatingActionMenuBase>
339339
);
340340
};
341+
342+
const ValueField = (props: {
343+
dynamicParameterModels: DynamicallyAddedParameterInputsModel[];
344+
schemaKey: string;
345+
onDynamicallyAddedParameterChange: (schemaKey: string, propertyName: string, newPropertyValue?: string) => void;
346+
}): JSX.Element => {
347+
const { dynamicParameterModels, schemaKey, onDynamicallyAddedParameterChange } = props;
348+
const modelIndex = dynamicParameterModels.findIndex((model) => model.schemaKey === schemaKey);
349+
const description = dynamicParameterModels[modelIndex].properties.description;
350+
const [valueField, setValueField] = useState(description);
351+
352+
const onDescriptionChange = (e: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>, newPropertyValue?: string) => {
353+
e.preventDefault();
354+
setValueField(newPropertyValue ?? '');
355+
};
356+
357+
const onDescriptionBlur = (): void => {
358+
onDynamicallyAddedParameterChange(schemaKey, 'description', valueField);
359+
};
360+
361+
return (
362+
<TextField
363+
className="msla-dynamic-added-param-value-TextField"
364+
value={valueField}
365+
onChange={onDescriptionChange}
366+
onBlur={onDescriptionBlur}
367+
/>
368+
);
369+
};

‎libs/designer-ui/src/lib/floatingactionmenu/floatingactionmenuoutputs/__test__/floatingactionmenuoutputs.spec.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ describe('ui/floatingactionmenuoutputs', () => {
9898
const { getByPlaceholderText } = render(<FloatingActionMenuOutputs {...minimal} onChange={onChange} />);
9999
const newText = 'New text';
100100
fireEvent.change(getByPlaceholderText('Enter a name'), { target: { value: newText } });
101+
expect(onChange).not.toHaveBeenCalled();
102+
fireEvent.blur(getByPlaceholderText('Enter a name'));
101103
const expectedOnChange = {
102104
value: minimal.initialValue,
103105
viewModel: {
@@ -132,6 +134,8 @@ describe('ui/floatingactionmenuoutputs', () => {
132134
const { getByPlaceholderText } = render(<FloatingActionMenuOutputs {...minimal} onChange={onChange} />);
133135
const newText = 'New text';
134136
fireEvent.change(getByPlaceholderText('Enter a description of the output'), { target: { value: newText } });
137+
expect(onChange).not.toHaveBeenCalled();
138+
fireEvent.blur(getByPlaceholderText('Enter a description of the output'));
135139
const expectedOnChange = {
136140
value: minimal.initialValue,
137141
viewModel: {

‎libs/designer-ui/src/lib/panel/panelheader/panelheadertitle.tsx

+2-1
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,15 @@ export const PanelHeaderTitle = ({
6666
};
6767

6868
const onTitleBlur = (): void => {
69-
handleTitleUpdate(newTitleValue || '');
7069
if (errorMessage) {
7170
onChange(validValue || '');
7271
setNewTitleValue(validValue);
7372
setErrorMessage('');
73+
handleTitleUpdate(validValue || '');
7474
} else {
7575
onBlur?.(titleBeforeBlur);
7676
setTitleBeforeBlur(newTitleValue ?? '');
77+
handleTitleUpdate(newTitleValue ?? '');
7778
}
7879
};
7980

‎libs/designer-ui/src/lib/searchabledropdown/index.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { useIntl } from 'react-intl';
77
export interface SearchableDropdownProps {
88
dropdownProps: Pick<IDropdownProps, 'options'> & Partial<Omit<IDropdownProps, 'onChange' | 'onDismiss' | 'onRenderItem'>>;
99
onItemSelectionChanged: (id: string, isSelected: boolean) => void;
10+
onDismiss?: () => void;
1011
labelId?: string;
1112
searchPlaceholderText?: string;
1213
showSearchItemThreshold?: number;
@@ -16,6 +17,7 @@ export interface SearchableDropdownProps {
1617
export const SearchableDropdown: FC<SearchableDropdownProps> = ({
1718
dropdownProps,
1819
onItemSelectionChanged,
20+
onDismiss,
1921
searchPlaceholderText,
2022
showSearchItemThreshold: showFilterItemThreshold,
2123
className,
@@ -63,6 +65,7 @@ export const SearchableDropdown: FC<SearchableDropdownProps> = ({
6365
}
6466
}}
6567
onDismiss={() => {
68+
onDismiss?.();
6669
conditionalVisibilityTempArray.forEach((parameterId) => {
6770
onItemSelectionChanged(parameterId, true);
6871
});

‎libs/designer/src/lib/core/actions/bjsworkflow/undoRedo.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ export const storeStateToUndoRedoHistory = createAsyncThunk(
1818
async (action: AnyAction, { dispatch, getState }): Promise<void> => {
1919
const rootState = getState() as RootState;
2020
const stateHistoryLimit = rootState.designerOptions.hostOptions.maxStateHistorySize || CONSTANTS.DEFAULT_MAX_STATE_HISTORY_SIZE;
21+
const idReplacements = rootState.workflow.idReplacements;
2122

22-
if (shouldSkipSavingStateToHistory(action, stateHistoryLimit)) {
23+
if (shouldSkipSavingStateToHistory(action, stateHistoryLimit, idReplacements)) {
2324
return;
2425
}
2526

‎libs/designer/src/lib/core/state/__test__/undoRedoSlice.spec.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ describe('undo redo slice reducers', () => {
1818
let mockInitialState: StateHistory = {
1919
past: [],
2020
future: [],
21-
stateHistoryItemIndex: -1,
21+
undoRedoClickToggle: 0,
2222
};
2323

2424
const getMockPayload = (mockState: StateHistoryItem) => {
@@ -47,7 +47,7 @@ describe('undo redo slice reducers', () => {
4747
mockInitialState = {
4848
past: [mockCompressedState2, mockCompressedState3],
4949
future: [mockCompressedState4],
50-
stateHistoryItemIndex: 2,
50+
undoRedoClickToggle: 2,
5151
};
5252
state = reducer(mockInitialState, saveStateToHistory(getMockPayload(mockCompressedState5)));
5353
expect(state.past).toEqual([mockCompressedState3, mockCompressedState5]);
@@ -58,22 +58,22 @@ describe('undo redo slice reducers', () => {
5858
const mockInitialState = {
5959
past: [mockCompressedState1, mockCompressedState2],
6060
future: [mockCompressedState3],
61-
stateHistoryItemIndex: -1,
61+
undoRedoClickToggle: 0,
6262
};
6363

6464
// Current state gets put into future and latest past state gets removed to be used for current state
6565
let state = reducer(mockInitialState, updateStateHistoryOnUndoClick({ compressedState: mockCompressedState4.compressedState }));
6666
expect(state.past).toEqual([mockCompressedState1]);
6767
expect(state.future).toEqual([mockCompressedState4, mockCompressedState3]);
68-
expect(state.stateHistoryItemIndex).toEqual(1);
68+
expect(state.undoRedoClickToggle).toEqual(1);
6969
expect(state.currentEditedPanelNode).toEqual('Initialize_Variable');
7070
expect(state.currentEditedPanelTab).toEqual(constants.PANEL_TAB_NAMES.PARAMETERS);
7171

7272
// On second undo click, state2 should be saved in future array with current panel details
7373
state = reducer(state, updateStateHistoryOnUndoClick({ compressedState: mockCompressedState2.compressedState }));
7474
expect(state.past).toEqual([]);
7575
expect(state.future).toEqual([mockCompressedState2, mockCompressedState4, mockCompressedState3]);
76-
expect(state.stateHistoryItemIndex).toEqual(0);
76+
expect(state.undoRedoClickToggle).toEqual(0);
7777
expect(state.currentEditedPanelNode).toEqual(undefined);
7878
expect(state.currentEditedPanelTab).toEqual(undefined);
7979
});
@@ -82,22 +82,22 @@ describe('undo redo slice reducers', () => {
8282
const mockInitialState = {
8383
past: [mockCompressedState1],
8484
future: [mockCompressedState2, mockCompressedState3],
85-
stateHistoryItemIndex: 2,
85+
undoRedoClickToggle: 0,
8686
};
8787

8888
// Current state gets put into past and first future state gets removed to be used for current state
8989
let state = reducer(mockInitialState, updateStateHistoryOnRedoClick({ compressedState: mockCompressedState4.compressedState }));
9090
expect(state.past).toEqual([mockCompressedState1, mockCompressedState4]);
9191
expect(state.future).toEqual([mockCompressedState3]);
92-
expect(state.stateHistoryItemIndex).toEqual(2);
92+
expect(state.undoRedoClickToggle).toEqual(1);
9393
expect(state.currentEditedPanelNode).toEqual('Initialize_Variable');
9494
expect(state.currentEditedPanelTab).toEqual(constants.PANEL_TAB_NAMES.PARAMETERS);
9595

9696
// On second redo click, state2 should be saved in past array with current panel details
9797
state = reducer(state, updateStateHistoryOnRedoClick({ compressedState: mockCompressedState2.compressedState }));
9898
expect(state.past).toEqual([mockCompressedState1, mockCompressedState4, mockCompressedState2]);
9999
expect(state.future).toEqual([]);
100-
expect(state.stateHistoryItemIndex).toEqual(3);
100+
expect(state.undoRedoClickToggle).toEqual(0);
101101
expect(state.currentEditedPanelNode).toEqual(undefined);
102102
expect(state.currentEditedPanelTab).toEqual(undefined);
103103
});

‎libs/designer/src/lib/core/state/undoRedo/undoRedoSelectors.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ export const useCanRedo = () => {
99
return useSelector((state: RootState) => state.undoRedo.future.length > 0);
1010
};
1111

12-
export const useStateHistoryItemIndex = () => {
13-
return useSelector((state: RootState) => state.undoRedo.stateHistoryItemIndex);
12+
export const useUndoRedoClickToggle = () => {
13+
return useSelector((state: RootState) => state.undoRedo.undoRedoClickToggle);
1414
};

‎libs/designer/src/lib/core/state/undoRedo/undoRedoSlice.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { StateHistory, StateHistoryItem } from './undoRedoTypes';
55
export const initialState: StateHistory = {
66
past: [],
77
future: [],
8-
stateHistoryItemIndex: -1,
8+
undoRedoClickToggle: 0,
99
};
1010

1111
export const undoRedoSlice = createSlice({
@@ -35,7 +35,7 @@ export const undoRedoSlice = createSlice({
3535
state.currentEditedPanelNode = state.past[state.past.length - 1].editedPanelNode;
3636
state.past = state.past.slice(0, state.past.length - 1);
3737
state.future = [newStateItem, ...state.future];
38-
state.stateHistoryItemIndex = state.past.length;
38+
state.undoRedoClickToggle = state.undoRedoClickToggle === 0 ? 1 : 0;
3939
},
4040
updateStateHistoryOnRedoClick: (state, action: PayloadAction<StateHistoryItem>) => {
4141
// Add current edited panel details to state being added to history
@@ -51,7 +51,7 @@ export const undoRedoSlice = createSlice({
5151
state.currentEditedPanelNode = state.future[0].editedPanelNode;
5252
state.future = state.future.slice(1);
5353
state.past = [...state.past, newStateItem];
54-
state.stateHistoryItemIndex = state.past.length;
54+
state.undoRedoClickToggle = state.undoRedoClickToggle === 0 ? 1 : 0;
5555
},
5656
},
5757
extraReducers: (builder) => {

‎libs/designer/src/lib/core/state/undoRedo/undoRedoTypes.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import {
66
addEdgeFromRunAfter,
77
addNode,
88
addSwitchCase,
9-
deleteNode,
10-
deleteSwitchCase,
119
moveNode,
1210
pasteNode,
1311
pasteScopeNode,
@@ -19,7 +17,8 @@ import {
1917
export interface StateHistory {
2018
past: StateHistoryItem[];
2119
future: StateHistoryItem[];
22-
stateHistoryItemIndex: number;
20+
// Toggle whenever undo/redo is clicked. This is needed to re-render parameter panel on undo/redo
21+
undoRedoClickToggle: number;
2322
// On undo/redo, we don't want to lose track of what the edited panel was for past/future state when we change the current state
2423
currentEditedPanelTab?: string;
2524
currentEditedPanelNode?: string;
@@ -41,15 +40,18 @@ export type UndoRedoPartialRootState = Pick<
4140
export const undoableWorkflowActionTypes = [
4241
addNode,
4342
moveNode,
44-
deleteNode,
4543
addSwitchCase,
46-
deleteSwitchCase,
4744
addForeachToNode.pending,
4845
pasteNode,
4946
pasteScopeNode,
5047
updateRunAfter,
5148
removeEdgeFromRunAfter,
5249
addEdgeFromRunAfter,
50+
/**
51+
* Following operations trigger state save outside of middleware:
52+
* 1. Delete node operations are tracked through DeleteModal since there are different delete actions for different node types
53+
* 2. updateParameterConditionalVisibility is tracked through settingsection to avoid storing multiple states on hide/show all
54+
*/
5355
].map((action) => action.type);
5456

5557
export const undoablePanelActionTypes = [

0 commit comments

Comments
 (0)