Skip to content

Commit 992aedc

Browse files
philippfrommephilippfromme
committed
fix: manage state outside React to avoid updates with stale element
Closes #1131
1 parent bd62701 commit 992aedc

File tree

2 files changed

+77
-206
lines changed

2 files changed

+77
-206
lines changed

‎src/render/BpmnPropertiesPanel.js‎

Lines changed: 6 additions & 191 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,3 @@
1-
import {
2-
useState,
3-
useMemo,
4-
useEffect,
5-
useCallback
6-
} from '@bpmn-io/properties-panel/preact/hooks';
7-
8-
import {
9-
find,
10-
isArray,
11-
reduce
12-
} from 'min-dash';
13-
141
import { FeelLanguageContext, PropertiesPanel } from '@bpmn-io/properties-panel';
152

163
import {
@@ -28,7 +15,6 @@ const DEFAULT_FEEL_LANGUAGE_CONTEXT = {
2815
* @param {Object} props
2916
* @param {djs.model.Base|Array<djs.model.Base>} [props.element]
3017
* @param {Injector} props.injector
31-
* @param { (djs.model.Base) => Array<PropertiesProvider> } props.getProviders
3218
* @param {Object} props.layoutConfig
3319
* @param {Object} props.descriptionConfig
3420
* @param {Object} props.tooltipConfig
@@ -38,194 +24,38 @@ const DEFAULT_FEEL_LANGUAGE_CONTEXT = {
3824
export default function BpmnPropertiesPanel(props) {
3925
const {
4026
element,
27+
groups,
4128
injector,
42-
getProviders,
43-
layoutConfig: initialLayoutConfig,
29+
layoutConfig,
4430
descriptionConfig,
4531
tooltipConfig,
4632
feelPopupContainer,
4733
getFeelPopupLinks
4834
} = props;
4935

50-
const canvas = injector.get('canvas');
51-
const elementRegistry = injector.get('elementRegistry');
5236
const eventBus = injector.get('eventBus');
5337
const translate = injector.get('translate');
5438

55-
const [ state, setState ] = useState({
56-
selectedElement: element
57-
});
58-
59-
const selectedElement = state.selectedElement;
60-
61-
/**
62-
* @param {djs.model.Base | Array<djs.model.Base>} element
63-
*/
64-
const _update = (element) => {
65-
66-
if (!element) {
67-
return;
68-
}
69-
70-
let newSelectedElement = element;
71-
72-
// handle labels
73-
if (newSelectedElement && newSelectedElement.type === 'label') {
74-
newSelectedElement = newSelectedElement.labelTarget;
75-
}
76-
77-
setState({
78-
...state,
79-
selectedElement: newSelectedElement
80-
});
81-
82-
// notify interested parties on property panel updates
83-
eventBus.fire('propertiesPanel.updated', {
84-
element: newSelectedElement
85-
});
86-
};
87-
88-
// (2) react on element changes
89-
90-
// (2a) selection changed
91-
useEffect(() => {
92-
const onSelectionChanged = (e) => {
93-
const { newSelection = [] } = e;
94-
95-
if (newSelection.length > 1) {
96-
return _update(newSelection);
97-
}
98-
99-
const newElement = newSelection[0];
100-
101-
const rootElement = canvas.getRootElement();
102-
103-
if (isImplicitRoot(rootElement)) {
104-
return;
105-
}
106-
107-
_update(newElement || rootElement);
108-
};
109-
110-
eventBus.on('selection.changed', onSelectionChanged);
111-
112-
return () => {
113-
eventBus.off('selection.changed', onSelectionChanged);
114-
};
115-
}, []);
116-
117-
// (2b) selected element changed
118-
useEffect(() => {
119-
const onElementsChanged = (e) => {
120-
const elements = e.elements;
121-
122-
const updatedElement = findElement(elements, selectedElement);
123-
124-
if (updatedElement && elementExists(updatedElement, elementRegistry)) {
125-
_update(updatedElement);
126-
}
127-
};
128-
129-
eventBus.on('elements.changed', onElementsChanged);
130-
131-
return () => {
132-
eventBus.off('elements.changed', onElementsChanged);
133-
};
134-
}, [ selectedElement ]);
39+
const selectedElement = element;
13540

136-
// (2c) root element changed
137-
useEffect(() => {
138-
const onRootAdded = (e) => {
139-
const element = e.element;
140-
141-
_update(element);
142-
};
143-
144-
eventBus.on('root.added', onRootAdded);
145-
146-
return () => {
147-
eventBus.off('root.added', onRootAdded);
148-
};
149-
}, [ selectedElement ]);
150-
151-
// (2d) provided entries changed
152-
useEffect(() => {
153-
const onProvidersChanged = () => {
154-
_update(selectedElement);
155-
};
156-
157-
eventBus.on('propertiesPanel.providersChanged', onProvidersChanged);
158-
159-
return () => {
160-
eventBus.off('propertiesPanel.providersChanged', onProvidersChanged);
161-
};
162-
}, [ selectedElement ]);
163-
164-
// (2e) element templates changed
165-
useEffect(() => {
166-
const onTemplatesChanged = () => {
167-
_update(selectedElement);
168-
};
169-
170-
eventBus.on('elementTemplates.changed', onTemplatesChanged);
171-
172-
return () => {
173-
eventBus.off('elementTemplates.changed', onTemplatesChanged);
174-
};
175-
}, [ selectedElement ]);
176-
177-
// (3) create properties panel context
17841
const bpmnPropertiesPanelContext = {
17942
selectedElement,
18043
injector,
18144
getService(type, strict) { return injector.get(type, strict); }
18245
};
18346

184-
// (4) retrieve groups for selected element
185-
const providers = getProviders(selectedElement);
186-
187-
const groups = useMemo(() => {
188-
return reduce(providers, function(groups, provider) {
189-
190-
// do not collect groups for multi element state
191-
if (isArray(selectedElement)) {
192-
return [];
193-
}
194-
195-
const updater = provider.getGroups(selectedElement);
196-
197-
return updater(groups);
198-
}, []);
199-
}, [ providers, selectedElement ]);
200-
201-
// (5) notify layout changes
202-
const [ layoutConfig, setLayoutConfig ] = useState(initialLayoutConfig || {});
203-
204-
const onLayoutChanged = useCallback((newLayout) => {
47+
const onLayoutChanged = (layoutConfig) => {
20548
eventBus.fire('propertiesPanel.layoutChanged', {
206-
layout: newLayout
49+
layout: layoutConfig
20750
});
208-
}, [ eventBus ]);
209-
210-
// React to external layout changes
211-
useEffect(() => {
212-
const cb = (e) => {
213-
const { layout } = e;
214-
setLayoutConfig(layout);
215-
};
216-
217-
eventBus.on('propertiesPanel.setLayout', cb);
218-
return () => eventBus.off('propertiesPanel.setLayout', cb);
219-
}, [ eventBus, setLayoutConfig ]);
51+
};
22052

221-
// (6) notify description changes
22253
const onDescriptionLoaded = (description) => {
22354
eventBus.fire('propertiesPanel.descriptionLoaded', {
22455
description
22556
});
22657
};
22758

228-
// (7) notify tooltip changes
22959
const onTooltipLoaded = (tooltip) => {
23060
eventBus.fire('propertiesPanel.tooltipLoaded', {
23161
tooltip
@@ -253,18 +83,3 @@ export default function BpmnPropertiesPanel(props) {
25383
</BpmnPropertiesPanelContext.Provider>
25484
);
25585
}
256-
257-
258-
// helpers //////////////////////////
259-
260-
function isImplicitRoot(element) {
261-
return element && element.isImplicit;
262-
}
263-
264-
function findElement(elements, element) {
265-
return find(elements, (e) => e === element);
266-
}
267-
268-
function elementExists(element, elementRegistry) {
269-
return element && elementRegistry.get(element.id);
270-
}

‎src/render/BpmnPropertiesPanelRenderer.js‎

Lines changed: 71 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
event as domEvent
1616
} from 'min-dom';
1717

18+
import { isArray, reduce } from 'min-dash';
19+
1820
const DEFAULT_PRIORITY = 1000;
1921

2022
/**
@@ -61,13 +63,72 @@ export default class BpmnPropertiesPanelRenderer {
6163
this.detach();
6264
});
6365

64-
eventBus.on('root.added', (event) => {
65-
const { element } = event;
66+
this._selectedElement = null;
67+
this._groups = [];
6668

67-
this._render(element);
68-
});
69-
}
69+
const update = () => {
70+
updateSelectedElement();
71+
updateGroups();
72+
73+
this._render();
74+
75+
eventBus.fire('propertiesPanel.updated', {
76+
element: this._selectedElement
77+
});
78+
};
79+
80+
const updateSelectedElement = () => {
81+
const canvas = injector.get('canvas'),
82+
selection = injector.get('selection');
83+
84+
const rootElement = canvas.getRootElement();
85+
86+
if (isImplicitRoot(rootElement)) {
87+
return;
88+
}
89+
90+
const selectedElements = selection.get();
91+
92+
if (selectedElements.length > 1) {
93+
this._selectedElement = selectedElements;
94+
} else if (selectedElements.length === 1) {
95+
this._selectedElement = selectedElements[0];
96+
} else {
97+
this._selectedElement = rootElement;
98+
}
99+
};
70100

101+
const updateGroups = () => {
102+
const providers = this._getProviders(this._selectedElement);
103+
104+
this._groups = reduce(providers, (groups, provider) => {
105+
if (!this._selectedElement || isArray(this._selectedElement)) {
106+
return [];
107+
}
108+
109+
const updater = provider.getGroups(this._selectedElement);
110+
111+
return updater(groups);
112+
}, []);
113+
};
114+
115+
const updateLayout = ({ layout }) => {
116+
this._layout = layout;
117+
118+
this._render();
119+
120+
eventBus.fire('propertiesPanel.updated', {
121+
element: this._selectedElement
122+
});
123+
};
124+
125+
eventBus.on('selection.changed', update);
126+
eventBus.on('element.changed', update);
127+
eventBus.on('elementTemplates.changed', update);
128+
eventBus.on('propertiesPanel.providersChanged', update);
129+
130+
eventBus.on('propertiesPanel.setLayout', updateLayout);
131+
}
71132

72133
/**
73134
* Attach the properties panel to a parent node.
@@ -158,22 +219,17 @@ export default class BpmnPropertiesPanelRenderer {
158219
return event.providers;
159220
}
160221

161-
_render(element) {
162-
const canvas = this._injector.get('canvas');
163-
164-
if (!element) {
165-
element = canvas.getRootElement();
166-
}
167-
168-
if (isImplicitRoot(element)) {
222+
_render() {
223+
if (isImplicitRoot(this._selectedElement)) {
169224
return;
170225
}
171226

172227
render(
173228
<BpmnPropertiesPanel
174-
element={ element }
229+
element={ this._selectedElement }
230+
groups={ this._groups }
231+
layout={ this._layout }
175232
injector={ this._injector }
176-
getProviders={ this._getProviders.bind(this) }
177233
layoutConfig={ this._layoutConfig }
178234
descriptionConfig={ this._descriptionConfig }
179235
tooltipConfig={ this._tooltipConfig }

0 commit comments

Comments
 (0)