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

Integrate template icons #108

Merged
merged 4 commits into from
Apr 5, 2022
Merged

Integrate template icons #108

merged 4 commits into from
Apr 5, 2022

Conversation

pinussilvestrus
Copy link
Contributor

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Apr 1, 2022
@pinussilvestrus pinussilvestrus force-pushed the 106-integrate-template-icons branch from bffad14 to 1652784 Compare April 1, 2022 12:50
@pinussilvestrus
Copy link
Contributor Author

This is waiting for a new properties panel release, but I think we can still wait for the other improvements to be merged, e.g. bpmn-io/bpmn-js-properties-panel#648

Maybe we can simply use this branch for the overall version bumps before the release /cc @MaxTru @marstamm

@barmac
Copy link
Contributor

barmac commented Apr 4, 2022

The way the icons are implemented means:

  1. I get both icons on canvas and in the properties panel header when I use camunda-bpmn-js
  2. Same as above if I install bpmn-js-properties-panel and @bpmn-io/element-templates-icons-renderer in a project without camunda-bpmn-js
  3. My properties panel displays the icon in the header but I cannot see it on canvas when I build custom bpmn-js project without @bpmn-io/element-templates-icons-renderer

Do we intend to keep (3)? Perhaps the icons renderer should be required as a part of the element templates module -> to be extracted to a separate element templates package in the future.

@pinussilvestrus
Copy link
Contributor Author

I thought about this one and given the current situation (element-templates as part of the properties panel) I found it strange to include rendering parts in the properties panel. However, I'd love to see the element templates be a standalone module. We should definitely tackle this in Q2 IMHO.

@MaxTru MaxTru marked this pull request as ready for review April 5, 2022 07:10
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Apr 5, 2022
@MaxTru MaxTru requested review from a team, Skaiir and barmac and removed request for a team April 5, 2022 07:11
@MaxTru
Copy link
Contributor

MaxTru commented Apr 5, 2022

@pinussilvestrus is out sick today, but this is super well prepared and the dependencies are met. I am marking this as ready for review.

@MaxTru MaxTru marked this pull request as draft April 5, 2022 07:14
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Apr 5, 2022
@barmac
Copy link
Contributor

barmac commented Apr 5, 2022

We need to release and integrate another alpha of bpmn-js-properties-panel to merge it.

@MaxTru
Copy link
Contributor

MaxTru commented Apr 5, 2022

Back to draft. I realized, we should also bump prop-panel to integrate the latest fix

@MaxTru
Copy link
Contributor

MaxTru commented Apr 5, 2022

Icons arrived
Screenshot_20220405_112843

@MaxTru MaxTru marked this pull request as ready for review April 5, 2022 09:37
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Apr 5, 2022
@MaxTru MaxTru requested a review from philippfromme April 5, 2022 09:37
@MaxTru MaxTru requested a review from nikku April 5, 2022 10:01
@MaxTru MaxTru marked this pull request as draft April 5, 2022 12:20
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Apr 5, 2022
@MaxTru
Copy link
Contributor

MaxTru commented Apr 5, 2022

Back to draft, waiting until we have https://camunda.slack.com/archives/CKGH9LR40/p1649155653797739 fixed

@marstamm marstamm marked this pull request as ready for review April 5, 2022 14:14
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Apr 5, 2022
@MaxTru
Copy link
Contributor

MaxTru commented Apr 5, 2022

Works with fix :-)
Screenshot_20220405_203603

@merge-me merge-me bot merged commit 3609053 into main Apr 5, 2022
@merge-me merge-me bot deleted the 106-integrate-template-icons branch April 5, 2022 18:36
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate template icons in Cloud Modeler
5 participants