Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render popup menu in .djs-container #728

Merged
merged 3 commits into from
Jan 5, 2023
Merged

Render popup menu in .djs-container #728

merged 3 commits into from
Jan 5, 2023

Conversation

smbea
Copy link
Contributor

@smbea smbea commented Jan 5, 2023

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Jan 5, 2023
@smbea smbea requested a review from nikku January 5, 2023 14:33
@nikku
Copy link
Member

nikku commented Jan 5, 2023

Regarding bpmn-io/bpmn-js#1795 (comment): Any reason we don't simply attach it to djs-container (again)? What is the use-case to make it configurable?

@smbea
Copy link
Contributor Author

smbea commented Jan 5, 2023

The reason would be to make it configurable generally speaking, but actually I don't have a concrete use case.
It's simpler and quicker to fix but if we don't need it, we can fix it at the root of it. Putting it back at djs-container would mean reverting the djs-parent breaking changes, right?

@nikku
Copy link
Member

nikku commented Jan 5, 2023

Putting it back at djs-container would mean reverting the djs-parent breaking changes, right?

Let's keep that (at least the class being present). No need to revert, causing another breaking change.

@smbea
Copy link
Contributor Author

smbea commented Jan 5, 2023

Makes sense to me. Feel free to have another look

@nikku
Copy link
Member

nikku commented Jan 5, 2023

Added a test case.

@smbea smbea merged commit e1327ca into develop Jan 5, 2023
@smbea smbea deleted the fix-popup-menu-parent branch January 5, 2023 20:06
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jan 5, 2023
@nikku nikku changed the title fix(popup-menu): use parent defined in config Render popup menu in .djs-container Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants