Skip to content

Commit 4ff4dd6

Browse files
barmacphilippfromme
barmac
authored and
philippfromme
committed
fix: manage state outside of preact to avoid updates with stale element
Related to bpmn-io/bpmn-js-properties-panel#1159
1 parent c0e4b47 commit 4ff4dd6

File tree

6 files changed

+243
-423
lines changed

6 files changed

+243
-423
lines changed

‎src/provider/camunda/CamundaPropertiesProvider.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ const CAMUNDA_PLATFORM_GROUPS = [
4343
export default class CamundaPropertiesProvider {
4444

4545
constructor(propertiesPanel, injector) {
46-
propertiesPanel.registerProvider(LOW_PRIORITY, this);
47-
4846
this._injector = injector;
47+
48+
propertiesPanel.registerProvider(LOW_PRIORITY, this);
4949
}
5050

5151
getGroups(element) {

‎src/provider/zeebe/ZeebePropertiesProvider.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ const LOW_PRIORITY = 500;
3535
export default class ZeebePropertiesProvider {
3636

3737
constructor(propertiesPanel, injector) {
38-
propertiesPanel.registerProvider(LOW_PRIORITY, this);
39-
4038
this._injector = injector;
39+
40+
propertiesPanel.registerProvider(LOW_PRIORITY, this);
4141
}
4242

4343
getGroups(element) {

‎src/render/DmnPropertiesPanel.js‎

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

153
import {
@@ -22,138 +10,25 @@ import { PanelPlaceholderProvider } from './PanelPlaceholderProvider';
2210
/**
2311
* @param {Object} props
2412
* @param {djs.model.Base|Array<djs.model.Base>} [props.element]
13+
* @param {Array} props.groups
2514
* @param {Injector} props.injector
26-
* @param { (djs.model.Base) => Array<PropertiesProvider> } props.getProviders
2715
* @param {Object} props.layoutConfig
2816
* @param {Object} props.descriptionConfig
2917
* @param {Object} props.tooltipConfig
3018
*/
3119
export default function DmnPropertiesPanel(props) {
3220
const {
3321
element,
22+
groups,
3423
injector,
35-
getProviders,
3624
layoutConfig,
3725
descriptionConfig,
3826
tooltipConfig
3927
} = props;
4028

41-
const canvas = injector.get('canvas');
42-
const elementRegistry = injector.get('elementRegistry');
4329
const eventBus = injector.get('eventBus');
4430

45-
const [ state, setState ] = useState({
46-
selectedElement: element
47-
});
48-
49-
const selectedElement = state.selectedElement;
50-
51-
/**
52-
* @param {djs.model.Base | Array<djs.model.Base>} element
53-
*/
54-
const _update = (element) => {
55-
56-
if (!element) {
57-
return;
58-
}
59-
60-
let newSelectedElement = element;
61-
62-
// handle labels
63-
if (newSelectedElement && newSelectedElement.type === 'label') {
64-
newSelectedElement = newSelectedElement.labelTarget;
65-
}
66-
67-
setState({
68-
...state,
69-
selectedElement: newSelectedElement
70-
});
71-
72-
// notify interested parties on property panel updates
73-
eventBus.fire('propertiesPanel.updated', {
74-
element: newSelectedElement
75-
});
76-
};
77-
78-
// (2) react on element changes
79-
80-
// (2a) selection changed
81-
useEffect(() => {
82-
const onSelectionChanged = (e) => {
83-
const { newSelection = [] } = e;
84-
85-
if (newSelection.length > 1) {
86-
return _update(newSelection);
87-
}
88-
89-
const newElement = newSelection[0];
90-
91-
const rootElement = canvas.getRootElement();
92-
93-
if (isImplicitRoot(rootElement)) {
94-
return;
95-
}
96-
97-
_update(newElement || rootElement);
98-
};
99-
100-
eventBus.on('selection.changed', onSelectionChanged);
101-
102-
return () => {
103-
eventBus.off('selection.changed', onSelectionChanged);
104-
};
105-
}, []);
106-
107-
// (2b) selected element changed
108-
useEffect(() => {
109-
const onElementsChanged = (e) => {
110-
const elements = e.elements;
111-
112-
const updatedElement = findElement(elements, selectedElement);
113-
114-
if (updatedElement && elementExists(updatedElement, elementRegistry)) {
115-
_update(updatedElement);
116-
}
117-
};
118-
119-
eventBus.on('elements.changed', onElementsChanged);
120-
121-
return () => {
122-
eventBus.off('elements.changed', onElementsChanged);
123-
};
124-
}, [ selectedElement ]);
125-
126-
// (2c) root element changed
127-
useEffect(() => {
128-
const onRootAdded = (e) => {
129-
const element = e.element;
130-
131-
if (isImplicitRoot(element)) {
132-
return;
133-
}
134-
135-
_update(element);
136-
};
137-
138-
eventBus.on('root.added', onRootAdded);
139-
140-
return () => {
141-
eventBus.off('root.added', onRootAdded);
142-
};
143-
}, [ selectedElement ]);
144-
145-
// (2d) provided entries changed
146-
useEffect(() => {
147-
const onProvidersChanged = () => {
148-
_update(selectedElement);
149-
};
150-
151-
eventBus.on('propertiesPanel.providersChanged', onProvidersChanged);
152-
153-
return () => {
154-
eventBus.off('propertiesPanel.providersChanged', onProvidersChanged);
155-
};
156-
}, [ selectedElement ]);
31+
const selectedElement = element;
15732

15833
// (3) create properties panel context
15934
const dmnPropertiesPanelContext = {
@@ -162,22 +37,6 @@ export default function DmnPropertiesPanel(props) {
16237
getService(type, strict) { return injector.get(type, strict); }
16338
};
16439

165-
// (4) retrieve groups for selected element
166-
const providers = getProviders(selectedElement);
167-
168-
const groups = useMemo(() => {
169-
return reduce(providers, function(groups, provider) {
170-
171-
// do not collect groups for multi element state
172-
if (isArray(selectedElement)) {
173-
return [];
174-
}
175-
const updater = provider.getGroups(selectedElement);
176-
177-
return updater(groups);
178-
}, []);
179-
}, [ providers, selectedElement ]);
180-
18140
// (5) notify layout changes
18241
const onLayoutChanged = (layout) => {
18342
eventBus.fire('propertiesPanel.layoutChanged', {
@@ -214,19 +73,4 @@ export default function DmnPropertiesPanel(props) {
21473
eventBus={ eventBus }
21574
/>
21675
</DmnPropertiesPanelContext.Provider>;
217-
}
218-
219-
220-
// helpers //////////////////////////
221-
222-
function isImplicitRoot(element) {
223-
return element && element.isImplicit;
224-
}
225-
226-
function findElement(elements, element) {
227-
return find(elements, (e) => e === element);
228-
}
229-
230-
function elementExists(element, elementRegistry) {
231-
return element && elementRegistry.get(element.id);
23276
}

‎src/render/DmnPropertiesPanelRenderer.js‎

Lines changed: 75 additions & 11 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
/**
@@ -51,13 +53,32 @@ export default class DmnPropertiesPanelRenderer {
5153
this.detach();
5254
});
5355

56+
this._selectedElement = null;
57+
this._groups = [];
58+
59+
eventBus.on('selection.changed', () => this._update());
60+
eventBus.on('elements.changed', () => this._update());
61+
eventBus.on('propertiesPanel.providersChanged', () => this._update());
62+
63+
// Handle root changes more carefully
64+
eventBus.on('root.added', (event) => {
65+
const element = event.element;
66+
67+
if (isImplicitRoot(element)) {
68+
return;
69+
}
70+
71+
this._update();
72+
});
73+
5474
eventBus.on('import.done', (event) => {
55-
const { element } = event;
5675

5776
if (parent) {
5877
this.attachTo(parent);
5978
}
60-
this._render(element);
79+
this._updateSelectedElement();
80+
this._updateGroups();
81+
this._render();
6182
});
6283

6384
eventBus.on('detach', (event) => {
@@ -151,20 +172,15 @@ export default class DmnPropertiesPanelRenderer {
151172
return event.providers;
152173
}
153174

154-
_render(element) {
155-
const canvas = this._injector.get('canvas');
156-
157-
if (!element) {
158-
element = canvas.getRootElement();
159-
}
160-
161-
if (isImplicitRoot(element)) {
175+
_render() {
176+
if (!this._selectedElement || isImplicitRoot(this._selectedElement)) {
162177
return;
163178
}
164179

165180
render(
166181
<DmnPropertiesPanel
167-
element={ element }
182+
element={ this._selectedElement }
183+
groups={ this._groups }
168184
injector={ this._injector }
169185
getProviders={ this._getProviders.bind(this) }
170186
layoutConfig={ this._layoutConfig }
@@ -184,6 +200,54 @@ export default class DmnPropertiesPanelRenderer {
184200
this._eventBus.fire('propertiesPanel.destroyed');
185201
}
186202
}
203+
204+
_update() {
205+
this._updateSelectedElement();
206+
this._updateGroups();
207+
208+
this._render();
209+
210+
this._eventBus.fire('propertiesPanel.updated', {
211+
element: this._selectedElement
212+
});
213+
}
214+
215+
_updateSelectedElement() {
216+
const canvas = this._injector.get('canvas');
217+
const rootElement = canvas.getRootElement();
218+
219+
if (isImplicitRoot(rootElement)) {
220+
this._selectedElement = rootElement;
221+
return;
222+
}
223+
224+
const selection = this._injector.get('selection');
225+
const selectedElements = selection.get();
226+
227+
if (selectedElements.length > 1) {
228+
this._selectedElement = selectedElements;
229+
} else if (selectedElements.length === 1) {
230+
this._selectedElement = selectedElements[0];
231+
} else {
232+
this._selectedElement = rootElement;
233+
}
234+
}
235+
236+
_updateGroups() {
237+
if (!this._selectedElement || isImplicitRoot(this._selectedElement) || isArray(this._selectedElement)) {
238+
this._groups = [];
239+
240+
return;
241+
}
242+
243+
const providers = this._getProviders(this._selectedElement);
244+
245+
this._groups = reduce(providers, (groups, provider) => {
246+
const updater = provider.getGroups(this._selectedElement);
247+
248+
return updater(groups);
249+
}, []);
250+
}
187251
}
188252

189253
DmnPropertiesPanelRenderer.$inject = [ 'config.propertiesPanel', 'injector', 'eventBus', '_parent' ];

0 commit comments

Comments
 (0)