fix: update on import done #128
Conversation
philippfromme
requested review from
Buckwich,
barmac,
Copilot and
jarekdanielak
There was a problem hiding this comment.
Pull request overview
This PR updates the DMN properties panel to listen to the import.done event instead of root.added, following the same fix previously implemented in bpmn-js-properties-panel (#1169). This change prevents stale element issues by ensuring the panel updates with the correct root element after import completion.
Key changes:
- Changed event listener from
root.addedtoimport.donein the properties panel - Removed implicit root check since
import.donefires with the actual root element - Updated test infrastructure to support canvas mocking
- Removed obsolete test for
root.addedbehavior
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/render/DmnPropertiesPanel.js | Updated useEffect to listen to import.done event instead of root.added, simplified logic by using canvas.getRootElement() directly |
| test/spec/DmnPropertiesPanel.spec.js | Removed test for old root.added behavior and added canvas mock setup to helper function |
| test/spec/mocks/index.js | Added support for overriding canvas mock via options |
| CHANGELOG.md | Documented the fix in version 3.8.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // then | ||
| expect(updateSpy).to.have.been.calledOnce; | ||
| }); |
There was a problem hiding this comment.
The test for the old root.added event behavior was removed, but no test was added for the new import.done event behavior. Consider adding a test to verify that the properties panel updates correctly when import.done is fired, similar to the removed test but checking for the import.done event instead.
Follow-up to bpmn-io/bpmn-js-properties-panel#1169 implementing the same fix in DMN properties panel.
Related to bpmn-io/bpmn-js-properties-panel#1131